Attachment 98402 Details for Bug 23290 – Bug 23290: (follow-up) Disable expand_entities unless explicitly enabled

[patch] Bug 23290: (follow-up) Disable expand_entities unless explicitly enabled

Bug-23290-follow-up-Disable-expandentities-unless-.patch (text/plain), 2.26 KB, created by Martin Renvoize (ashimema) on 2020-02-04 14:27:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2020-02-04 14:27:18 UTC

Size: 2.26 KB

patch

obsolete

>From da2c4b12995689c3286c494683b6ecbd416fd735 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 17 Jan 2020 11:01:14 +0000
>Subject: [PATCH] Bug 23290: (follow-up) Disable expand_entities unless
> explicitly enabled
>
>This follow-up refines the change made in the former patch.
>
>See also
>https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=838097
>https://rt.cpan.org/Public/Bug/Display.html?id=118032
>
>We do not want to depend now on the exact LibXML version, so we will
>disable expand_entities unless it is explicitly enabled via the config
>variable koha_xslt_security. (Allowing us to test if bad things will be
>caught.)
>
>The options key is now always added to the Security object.
>The return from set_parser_options has been removed to allow disabling when
>there is no koha-conf entry (which probably is the normal situation).
>
>Test plan:
>[1] Test the first example patch with and without the other patches (excl.
>    the second example). Toggle expand_entities in koha-conf. Restart
>    Plack and flush the cache each time. Evaluate results with the
>    commit message of first example.
>[2] Test both example patches with/without other patches.
>    Toggle expand_entities. Restart etc. Evaluate results with commit
>    message of second example (check tmp/breached.txt).
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> Koha/XSLT/Security.pm | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/Koha/XSLT/Security.pm b/Koha/XSLT/Security.pm
>index 6c75458b02..55cbaddf3d 100644
>--- a/Koha/XSLT/Security.pm
>+++ b/Koha/XSLT/Security.pm
>@@ -55,6 +55,7 @@ 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;
>@@ -142,11 +143,11 @@ sub set_callbacks {
> sub set_parser_options {
>     my ($self, $parser) = @_;
>     my $conf = $self->{_options};
>-    return if !$conf;
> 
>     if( $conf->{expand_entities} ) {
>         _set_option($parser, 'expand_entities', 1);
>     } else {
>+        # If not explicitly set, we should disable expanding for security
>         _set_option($parser, 'expand_entities', 0);
>     }
> }
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 23290: 91435 | 91437 | 95450 | 95451 | 95452 | 95453 | 95454 | 95475 | 95478 | 95480 | 95481 | 95482 | 95483 | 95484 | 95485 | 95551 | 96299 | 96300 | 96301 | 96302 | 96303 | 96304 | 97606 | 97607 | 97608 | 97609 | 97610 | 97611 | 97612 | 97613 | 97614 | 97615 | 97616 | 97617 | 97618 | 97619 | 97620 | 97871 | 97978 | 97980 | 98397 | 98398 | 98399 | 98400 | 98401 | 98402 | 98403 | 98404 | 98405 | 98406 | 99587