diff --git a/Koha/XSLT/Security.pm b/Koha/XSLT/Security.pm new file mode 100644 index 0000000000..8354227307 --- /dev/null +++ b/Koha/XSLT/Security.pm @@ -0,0 +1,170 @@ +package Koha::XSLT::Security; + +# Copyright 2019 Prosentient Systems, Rijksmuseum +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +=head1 NAME + +Koha::XSLT::Security - Add security features to Koha::XSLT_Handler + +=head1 SYNOPSIS + + use Koha::XSLT::Security; + my $secu = Koha::XSLT::Security->new; + $secu->register_callbacks; + $secu->set_parser_options($parser); + +=head1 DESCRIPTION + + This object allows you to apply security options to Koha::XSLT_Handler. + It looks for parser options in koha-conf.xml. + +=cut + +use Modern::Perl; +use Data::Dumper qw/Dumper/; +use XML::LibXSLT; +use C4::Context; + +use base qw(Class::Accessor); + +=head1 METHODS + +=head2 new + + Creates object, checks if koha-conf.xml contains additional configuration + options, and checks if XML::LibXSLT::Security is present. + +=cut + +sub new { + my ($class) = @_; + my $self = {}; + + $self->{_options} = {}; + my $conf = C4::Context->config('koha_xslt_security'); + if( $conf && ref($conf) eq 'HASH' ) { + $self->{_options} = $conf; + } + + my $security = eval { XML::LibXSLT::Security->new }; + if( $security ) { + $self->{_security_obj} = $security; + } else { + warn "No XML::LibXSLT::Security object: $@"; #TODO Move to about ? + } + + return bless $self, $class; +} + +=head2 register_callbacks + + Register LibXSLT security callbacks + +=cut + +sub register_callbacks { + my $self = shift; + + my $security = $self->{_security_obj}; + return if !$security; + + $security->register_callback( read_file => sub { + warn "read_file called in XML::LibXSLT"; + #i.e. when using the exsl:document() element or document() function (to read a XML file) + my ($tctxt,$value) = @_; + return 0; + }); + $security->register_callback( write_file => sub { + warn "write_file called in XML::LibXSLT"; + #i.e. when using the exsl:document element (or document() function?) (to write an output file of many possible types) + #e.g. + # + # breached! + # + my ($tctxt,$value) = @_; + return 0; + }); + $security->register_callback( read_net => sub { + warn "read_net called in XML::LibXSLT"; + #i.e. when using the document() function (to read XML from the network) + #e.g. + my ($tctxt,$value) = @_; + return 0; + }); + $security->register_callback( write_net => sub { + warn "write_net called in XML::LibXSLT"; + #NOTE: it's unknown how one would invoke this, but covering our bases anyway + my ($tctxt,$value) = @_; + return 0; + }); +} + +=head2 set_callbacks + + my $xslt = XML::LibXSLT->new; + $security->set_callbacks( $xslt ); + + Apply registered callbacks to a specific xslt instance. + +=cut + +sub set_callbacks { + my ($self, $xslt) = @_; + + my $security = $self->{_security_obj}; + return if !$security; + $xslt->security_callbacks( $security ); +} + +=head2 set_parser_options + + $security->set_parser_options($parser); + + If koha-conf.xml includes koha_xslt_security options, set them. + We start with implementing expand_entities. + +=cut + +sub set_parser_options { + my ($self, $parser) = @_; + my $conf = $self->{_options}; + + if( $conf->{expand_entities_unsafe} ) { # NOT recommended + _set_option($parser, 'expand_entities', 1); + } else { + # If not explicitly set, we should disable expanding for security + _set_option($parser, 'expand_entities', 0); + } +} + +sub _set_option { + my ($parser, $option_name, $value) = @_; + if( $parser->option_exists($option_name) ) { + $parser->set_option($option_name, $value); + } + #TODO Should we warn if it does not exist? +} + +=head1 AUTHOR + + David Cook, Prosentient Systems + Marcel de Rooy, Rijksmuseum Netherlands + +=cut + +1; diff --git a/Koha/XSLT_Handler.pm b/Koha/XSLT_Handler.pm index 5b0726fddf..3d97f8ceae 100644 --- a/Koha/XSLT_Handler.pm +++ b/Koha/XSLT_Handler.pm @@ -1,6 +1,6 @@ package Koha::XSLT_Handler; -# Copyright 2014 Rijksmuseum +# Copyright 2014, 2019 Rijksmuseum, Prosentient Systems # # This file is part of Koha. # @@ -43,7 +43,7 @@ Koha::XSLT_Handler - Facilitate use of XSLT transformations =head2 new - Create handler object (via Class::Accessor) + Create handler object =head2 transform @@ -118,6 +118,7 @@ Koha::XSLT_Handler - Facilitate use of XSLT transformations use Modern::Perl; use XML::LibXML; use XML::LibXSLT; +use Koha::XSLT::Security; use base qw(Class::Accessor); @@ -132,6 +133,20 @@ use constant XSLTH_ERR_5 => 'XSLTH_ERR_PARSING_DATA'; use constant XSLTH_ERR_6 => 'XSLTH_ERR_TRANSFORMING'; use constant XSLTH_ERR_7 => 'XSLTH_NO_STRING_PASSED'; +=head2 new + + my $xslt_engine = Koha::XSLT_Handler->new; + +=cut + +sub new { + my ($class, $params) = @_; + my $self = $class->SUPER::new($params); + $self->{_security} = Koha::XSLT::Security->new; + $self->{_security}->register_callbacks; + return $self; +} + =head2 transform my $output= $xslt_engine->transform( $xml, $xsltfilename, [$format] ); @@ -195,6 +210,7 @@ sub transform { #parse input and transform my $parser = XML::LibXML->new(); + $self->{_security}->set_parser_options($parser); my $source = eval { $parser->parse_string($xml) }; if ($@) { $self->_set_error( XSLTH_ERR_5, $@ ); @@ -310,6 +326,7 @@ sub _load { #load sheet my $parser = XML::LibXML->new; + $self->{_security}->set_parser_options($parser); my $style_doc = eval { $parser->load_xml( $self->_load_xml_args($filename, $code) ) }; @@ -320,6 +337,8 @@ sub _load { #parse sheet my $xslt = XML::LibXSLT->new; + $self->{_security}->set_callbacks($xslt); + $rv = $code? $digest.$codelen: $filename; $self->{xslt_hash}->{$rv} = eval { $xslt->parse_stylesheet($style_doc) }; if ($@) { @@ -348,6 +367,7 @@ sub _set_error { =head1 AUTHOR Marcel de Rooy, Rijksmuseum Netherlands + David Cook, Prosentient Systems =cut diff --git a/t/db_dependent/Koha/XSLT/Security.t b/t/db_dependent/Koha/XSLT/Security.t new file mode 100644 index 0000000000..249d549afc --- /dev/null +++ b/t/db_dependent/Koha/XSLT/Security.t @@ -0,0 +1,132 @@ +#!/usr/bin/perl + +# Copyright 2019 Rijksmuseum +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; +use File::Temp qw/tempfile/; +use Test::More tests => 7; +use Test::Warn; + +use Koha::XSLT_Handler; +use t::lib::Mocks; + +t::lib::Mocks::mock_config( 'koha_xslt_security', { expand_entities_unsafe => 1 } ); +my $engine=Koha::XSLT_Handler->new; + +my $secret_file = mytempfile('Big secret'); +my $xslt=<<"EOT"; +]> + + + &secret; + + + + +EOT +my $xslt_file = mytempfile($xslt); + +my $output= $engine->transform( "", $xslt_file ); +like($output, qr/Big secret/, 'external entity got through'); + +t::lib::Mocks::mock_config( 'koha_xslt_security', { expand_entities_unsafe => 0 } ); +$engine=Koha::XSLT_Handler->new; +$output= $engine->transform( "", $xslt_file ); +unlike($output, qr/Big secret/, 'external entity did not get through'); + +# Adding a document call to trigger callback for read_file +# Does not depend on expand_entities. +$xslt=<<"EOT"; + + + + + + +EOT +$xslt_file = mytempfile($xslt); +warnings_like { $output= $engine->transform( "", $xslt_file ); } + [ qr/read_file called in XML::LibXSLT/, qr/runtime error/ ], + 'Triggered security callback for read_file'; + +# Trigger write_file +$xslt=<<"EOT"; + + + + Breached! + + +EOT +$xslt_file = mytempfile($xslt); +warnings_like { $output= $engine->transform( "", $xslt_file ); } + [ qr/write_file called in XML::LibXSLT/, qr/runtime error/ ], + 'Triggered security callback for write_file'; + +# Trigger read_net +$xslt=<<"EOT"; + + + + + + +EOT +$xslt_file = mytempfile($xslt); +warnings_like { $output= $engine->transform( "", $xslt_file ); } + [ qr/read_net called in XML::LibXSLT/, qr/runtime error/ ], + 'Triggered security callback for read_net'; + +# Trigger write_net +$xslt=<<"EOT"; + + + + + Breached! + + + +EOT +$xslt_file = mytempfile($xslt); +warnings_like { $output= $engine->transform( "", $xslt_file ); } + [ qr/write_net called in XML::LibXSLT/, qr/runtime error/ ], + 'Triggered security callback for write_net'; + +# Check remote import (include should be similar) +# Trusting koha-community.org DNS here ;) +# This should not trigger read_net but fail on the missing import. +$xslt=<<"EOT"; + + + + + +EOT +$xslt_file = mytempfile($xslt); +$engine->print_warns(1); +warning_like { $output= $engine->transform( "", $xslt_file ); } + qr/I\/O warning : failed to load external entity/, + 'Remote import does not fail on read_net'; + +sub mytempfile { + my ( $fh, $fn ) = tempfile( SUFFIX => '.xsl', UNLINK => 1 ); + print $fh $_[0]//''; + close $fh; + return $fn; +}