[Rt-commit] rt branch, 4.2/html-gumbo-balancing, created. rt-4.2.2-6-gdc922ea

[Alex Vandiver]

The branch, 4.2/html-gumbo-balancing has been created
        at  dc922eab9e5c3fd8b7ab15afc61fe258d8cc530a (commit)

- Log -----------------------------------------------------------------
commit 6c0cbbbd1b6171e666b860704e0aee11d462e154
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 15 16:33:52 2014 -0500

    Allow tables if HTML::Gumbo is installed
    
    HTML::Gumbo deals with ensuring that content cannot "escape" from the
    context that RT frames it in, by (for example) not allowing </td></tr>
    if the content has not opened its own table.  HTML::Gumbo has an
    HTML::Parser-like interface, but it is not quite close enough to serve
    as a drop-in replacement -- and the structure of HTML::Scrubber would
    not make such a substitution easy.
    
    As such, pre-parse the HTML content using Gumbo, if available, as a
    pre-parsing step before HTML::Scrubber.  This enables <table> tags and
    their ilk to be enabled without posing a security risk.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 0aebeed..f666ca4 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -3868,6 +3868,22 @@ if (RT->Config->Get('ShowTransactionImages') or RT->Config->Get('ShowRemoteImage
 sub _NewScrubber {
     require HTML::Scrubber;
     my $scrubber = HTML::Scrubber->new();
+
+    if (eval "require HTML::Gumbo; 1") {
+        no warnings 'redefine';
+        my $orig = \&HTML::Scrubber::scrub;
+        *HTML::Scrubber::scrub = sub {
+            my $self = shift;
+
+            eval { $_[0] = HTML::Gumbo->new->parse( $_[0] ); chomp $_[0] };
+            warn "HTML::Gumbo pre-parse failed: $@" if $@;
+            return $orig->($self, @_);
+        };
+        push @SCRUBBER_ALLOWED_TAGS, qw/TABLE THEAD TBODY TFOOT TR TD TH/;
+        $SCRUBBER_ALLOWED_ATTRIBUTES{$_} = 1 for
+            qw/colspan rowspan align valign cellspacing cellpadding border width height/;
+    }
+
     $scrubber->default(
         0,
         {

commit bb68df313ca7bd9a19a21b6f19e55abdef5421e5
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 15 16:42:47 2014 -0500

    Allow additional CSS rules for borders and padding

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index f666ca4..a8d7f05 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -3836,6 +3836,12 @@ our %SCRUBBER_ALLOWED_ATTRIBUTES = (
                font-family: \s* [\w\s"',.\-]+       |
                font-weight: \s* [\w\-]+             |
 
+               border-style: \s* \w+                |
+               border-color: \s* [#\w]+             |
+               border-width: \s* [\s\w]+            |
+               padding: \s* [\s\w]+                 |
+               margin: \s* [\s\w]+                  |
+
                # MS Office styles, which are probably fine.  If we don't, then any
                # associated styles in the same attribute get stripped.
                mso-[\w\-]+?: \s* [\w\s"',.\-]+

commit dc922eab9e5c3fd8b7ab15afc61fe258d8cc530a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 15 16:48:37 2014 -0500

    Document that RT scrubs HTML, and note that HTML::Gumbo loosens those slightly

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index 309601d..39b8071 100755
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -1764,10 +1764,18 @@ Set($AlwaysDownloadAttachments, undef);
 
 =item C<$PreferRichText>
 
-By default, RT shows rich text (HTML) messages if possible.
-
-If C<$PreferRichText> is set to 0, RT will show plain text messages
-in preference to any rich text alternatives.
+By default, RT shows rich text (HTML) messages if possible.  If
+C<$PreferRichText> is set to 0, RT will show plain text messages in
+preference to any rich text alternatives.
+
+As a security precaution, RT limits the HTML that is displayed to a
+known-good subset -- as allowing arbitrary HTML to be displayed exposes
+multiple vectors for XSS and phishing attacks.  If
+L</$TrustHTMLAttachments> is enabled, the original HTML is available for
+viewing via the "Download" link.
+
+If the optional L<HTML::Gumbo> dependency is installed, RT will leverage
+this to allow a broader set of HTML through, including tables.
 
 =cut
 

-----------------------------------------------------------------------