Merge branch 'rel-10_1' into rel-11_0

RotherOSS · Mar 27, 2024 · 12b3b21 · 12b3b21
2 parents 9d617d9 + fb3a6ac
commit 12b3b21
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 28 deletions.
diff --git a/Kernel/System/HTMLUtils.pm b/Kernel/System/HTMLUtils.pm
@@ -1078,6 +1078,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. <SCRIPT/XSS SRC="http://xss.rocks/xss.js"></SCRIPT>
+        # 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)) {
@@ -1110,14 +1119,6 @@ sub Safety {
 
         if ( $Param{NoJavaScript} ) {
 
-            # consider non-alpha, non-digit chars in the tag as suspicious
-            # e.g. <SCRIPT/XSS SRC="http://xss.rocks/xss.js"></SCRIPT>
-            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/Frontend/PictureUpload.t b/scripts/test/Frontend/PictureUpload.t
@@ -165,16 +165,16 @@ my $ContentSVG = <<'EOF';
 <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
 
 <svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
-<polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/>
+<polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke='#004400'/>
 <script type="text/javascript">alert(document.domain);</script></svg>
 EOF
 
 my $EscapedContentSVG = <<'EOF';
-<?xml version="1.0" standalone="no"?>
-<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
 
-<svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
-<polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/>
+
+
+<svg version="1.1" baseprofile="full" xmlns="http://www.w3.org/2000/svg">
+<polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400" />
 </svg>
 EOF
 

diff --git 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',
 <img/src="http://example.com/image.png"/>
 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="http://example.com/image.png"/>
+img/src:<img/src="http://example.com/image.png"/>filtered out
 EOF
         Config => {
             NoImg => 1,
@@ -950,25 +950,41 @@ EOF
 
             # note the inserted space befor '/>'
             Output => <<'EOF',
-img/src:<img/src="http://example.com/image.png" />
+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:<img/src="http://example.com/image.png"/>
+line1:<img/src="http://example.com/image.png"/>filtered out
 line2:
 EOF
         Config => {
             NoJavaScript => 1,
         },
         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:<img/src="http://example.com/image.png"/>filtered out without parameters
+line2:
+EOF
+        Config => {
+        },
+        Result => {
+            Output => <<'EOF',
+line1:filtered out without parameters
 line2:
 EOF
             Replace => 1,
@@ -1268,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',
 <?xml version="1.0" standalone="no"?>
 <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
 
 <svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
-<polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/>
+<polygon id="triangle" points="0,0 0,50 50,0" fill='#009900' stroke="#004400"/>
 </svg>
 END_SVG
         Config => {
@@ -1289,11 +1304,11 @@ END_SVG
         Result => {
             Replace => 0,
             Output  => <<'END_SVG',
-<?xml version="1.0" standalone="no"?>
-<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
 
-<svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
-<polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/>
+
+
+<svg version="1.1" baseprofile="full" xmlns="http://www.w3.org/2000/svg">
+<polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400" />
 </svg>
 END_SVG
         },