From 7b192d74ac04d9d78ff14dc105cc424460005044 Mon Sep 17 00:00:00 2001 From: bernhard Date: Wed, 27 Mar 2024 11:01:30 +0100 Subject: [PATCH 1/3] Issue #3249: always filter out tags like 'img/src' --- Kernel/System/HTMLUtils.pm | 17 +++++++++-------- scripts/test/HTMLUtils/Safety.t | 34 ++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/Kernel/System/HTMLUtils.pm b/Kernel/System/HTMLUtils.pm index 35ddd41dd3..97511651be 100644 --- a/Kernel/System/HTMLUtils.pm +++ b/Kernel/System/HTMLUtils.pm @@ -1064,6 +1064,15 @@ sub Safety { # only inspect start tags return unless $Event eq 'start'; + # Consider non-alpha, non-digit chars in the tag as suspicious + # e.g. + # This filter is always active. + if ( $Tag =~ m/[^a-zA-Z0-9]/ ) { + $ScrubberReplaced++; + + return ''; # discard the tag + } + if ( $Param{NoIntSrcLoad} || $Param{NoExtSrcLoad} ) { BLACK_LISTED: for my $Blacklisted (qw(src poster)) { @@ -1096,14 +1105,6 @@ sub Safety { if ( $Param{NoJavaScript} ) { - # consider non-alpha, non-digit chars in the tag as suspicious - # e.g. - if ( $Tag =~ m/[^a-zA-Z0-9]/ ) { - $ScrubberReplaced++; - - return ''; # discard the tag - } - # remove HTTP redirects in meta tags if ( $Tag eq 'meta' && $Attr->{'http-equiv'} && $Attr->{'http-equiv'} =~ m/refresh/i ) { $ScrubberReplaced++; diff --git a/scripts/test/HTMLUtils/Safety.t b/scripts/test/HTMLUtils/Safety.t index ccc83d1fda..c6518642f6 100644 --- a/scripts/test/HTMLUtils/Safety.t +++ b/scripts/test/HTMLUtils/Safety.t @@ -885,6 +885,7 @@ EOF Line => __LINE__, }, { + Name => 'external image with / in tab name will be filtered', Input => <<'EOF', EOF @@ -894,7 +895,6 @@ EOF EOF Replace => 1, }, - Name => 'external image with / separator', Line => __LINE__, }, { @@ -939,9 +939,9 @@ for my $Test (@TestsWithDefaultConfig) { my @TestsWithExplicitConfig = ( { - Name => 'strange img tag "img/src" passes as NoJavaScript is not passed', + Name => 'tag "img/src" filtered out even when not recognized as image', Input => <<'EOF', -img/src: +img/src:filtered out EOF Config => { NoImg => 1, @@ -950,17 +950,16 @@ EOF # note the inserted space befor '/>' Output => <<'EOF', -img/src: +img/src:filtered out EOF - Replace => 0, + Replace => 1, }, Line => __LINE__, }, { - # Todo: that NoJavaScript is needed to filter out strange tags does not make sense - Name => 'strange img tag "img/src" is filtered out as NoJavaScript is passed', + Name => 'tag "img/src" is filtered out when NoJavaScript is passed', Input => <<'EOF', -line1: +line1:filtered out line2: EOF Config => { @@ -968,7 +967,24 @@ EOF }, Result => { Output => <<'EOF', -line1: +line1:filtered out +line2: +EOF + Replace => 1, + }, + Line => __LINE__, + }, + { + Name => 'tag "img/src" is filtered out even without parameters', + Input => <<'EOF', +line1:filtered out without parameters +line2: +EOF + Config => { + }, + Result => { + Output => <<'EOF', +line1:filtered out without parameters line2: EOF Replace => 1, From c1f4d4dabfe7e979bd7dce12be5e5e0888b23cc7 Mon Sep 17 00:00:00 2001 From: bernhard Date: Wed, 27 Mar 2024 11:15:17 +0100 Subject: [PATCH 2/3] Issue #3249: accept that DOCTYPE is removed --- scripts/test/HTMLUtils/Safety.t | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/scripts/test/HTMLUtils/Safety.t b/scripts/test/HTMLUtils/Safety.t index c6518642f6..95bf089678 100644 --- a/scripts/test/HTMLUtils/Safety.t +++ b/scripts/test/HTMLUtils/Safety.t @@ -1284,13 +1284,12 @@ You should be able to continue reading these lessons, however. { # svg attachments might contain XML declaration and DOCTYPE declaration Name => 'svg with XML and DOCTYPE declarations', - Todo => 'it is not clear how to handle declarations in the PictureUpload frontend', Input => <<'END_SVG', - + END_SVG Config => { @@ -1305,11 +1304,11 @@ END_SVG Result => { Replace => 0, Output => <<'END_SVG', - - - - + + + + END_SVG }, From 055b6ba2d18554d13ea5aa2a13b616cd0ec36095 Mon Sep 17 00:00:00 2001 From: bernhard Date: Wed, 27 Mar 2024 11:18:39 +0100 Subject: [PATCH 3/3] Issue #3249: adapt test to changed behavoir XML and DOCTYPE declarations are skipped when Safety() is implemented with HTML::Scrubber --- scripts/test/Frontend/PictureUpload.t | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/test/Frontend/PictureUpload.t b/scripts/test/Frontend/PictureUpload.t index 6f958f61ab..fcc2e35804 100644 --- a/scripts/test/Frontend/PictureUpload.t +++ b/scripts/test/Frontend/PictureUpload.t @@ -165,16 +165,16 @@ my $ContentSVG = <<'EOF'; - + EOF my $EscapedContentSVG = <<'EOF'; - - - - + + + + EOF