[pmg-devel] [PATCH pmg-api] fix #3734: scrub 'url' from style tags/attributes

[Dominik Csapak <d.csapak@proxmox.com>]

if 'view images' for the quarantine is disabled, it is expected that
*no* images will be loaded. but in addition to img (src/href/etc.)
also css can load external images via the 'url' directive

since html scrubber does not parse/iterate over css, we simply remove
the url+protocol part of those tags/attributes. this technically leaves behind
invalid css, but the browsers should cope with that.
(we cannot 'cleanly' remove without much more effort because of quoting)

also we have to scrub the style tags in 'dump_html' since HTML::Scrubber
does not have a way to modify the *content* of a tag, only the
attributes...

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/HTMLMail.pm | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/PMG/HTMLMail.pm b/src/PMG/HTMLMail.pm
index b69a596..987dc39 100644
--- a/src/PMG/HTMLMail.pm
+++ b/src/PMG/HTMLMail.pm
@@ -15,8 +15,16 @@ use HTML::Scrubber;
 use PMG::Utils;
 use PMG::MIMEUtils;
 
+# $value is a ref to a string scalar
+my sub remove_urls {
+    my ($value) = @_;
+    # remove all urls with a protocol, this leaves partially invalid
+    # css, but prevents the browser from loading them
+    $$value =~ s|url\s*\(\s*(['"]?)[a-z]+://|($1|gi;
+}
+
 sub dump_html {
-    my ($tree, $cid_hash) = @_;
+    my ($tree, $cid_hash, $viewimages) = @_;
 
     my @html = ();
 
@@ -37,6 +45,11 @@ sub dump_html {
 			    $node->{src} = $datauri;
 			}
 		    }
+		} elsif ($tag eq 'style' && !$viewimages) {
+		    for my $el ($node->content_refs_list()) {
+			next if ref $$el;
+			remove_urls($el);
+		    }
 		}
 
 		if($start) { # on the way in
@@ -137,7 +150,13 @@ sub getscrubber {
 	    span => 1,
 	    src => $viewimages ? qr{^(?!(?:java)?script)}i : 0,
 	    start => 1,
-	    style => 1,
+	    style => $viewimages ? 1 : sub {
+		my ($obj, $tag_name, $attr_name, $value) = @_;
+
+		remove_urls(\$value);
+
+		return $value;
+	    },
 	    summary => 1,
 	    tabindex => 1,
 	    target => 1,
@@ -267,7 +286,7 @@ sub entity_to_html {
 	$tree->parse($raw);
 	$tree->eof();
 
-	my $whtml = dump_html($tree, $viewimages ? $cid_hash : {});
+	my $whtml = dump_html($tree, $viewimages ? $cid_hash : {}, $viewimages); #scrubs style tags
 	$tree->delete;
 
 	# remove dangerous/unneeded elements
-- 
2.30.2