From ffb32485ca723ea7d44795a4f6e494ad9ebc6dd5 Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manishearth@gmail.com>
Date: Thu, 17 Apr 2014 18:46:39 +0200
Subject: [PATCH] Bug 968576: [SECURITY] Dangerous control characters allowed
 in Bugzilla text r=glob a=justdave

---
 Bugzilla/Template.pm                              | 11 +++++++++++
 Bugzilla/Util.pm                                  |  4 ++++
 template/en/default/email/newchangedmail.txt.tmpl |  2 +-
 template/en/default/request/email.txt.tmpl        |  2 +-
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index c8f606b987..ca551fa483 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -657,6 +657,17 @@ sub create {
                 my ($data) = @_;
                 return encode_base64($data);
             },
+
+            # Strips out control characters excepting whitespace
+            strip_control_chars => sub {
+                my ($data) = @_;
+                # Only run for utf8 to avoid issues with other multibyte encodings 
+                # that may be reassigning meaning to ascii characters.
+                if (Bugzilla->params->{'utf8'}) {
+                    $data =~ s/(?![\t\r\n])[[:cntrl:]]//g;
+                }
+                return $data;
+            },
             
             # HTML collapses newlines in element attributes to a single space,
             # so form elements which may have whitespace (ie comments) need
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index e83893d9a8..652f863d5e 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -88,6 +88,10 @@ sub html_quote {
     # Obscure '@'.
     $var =~ s/\@/\&#64;/g;
     if (Bugzilla->params->{'utf8'}) {
+        # Remove control characters if the encoding is utf8.
+        # Other multibyte encodings may be using this range; so ignore if not utf8.
+        $var =~ s/(?![\t\r\n])[[:cntrl:]]//g;
+
         # Remove the following characters because they're
         # influencing BiDi:
         # --------------------------------------------------------
diff --git a/template/en/default/email/newchangedmail.txt.tmpl b/template/en/default/email/newchangedmail.txt.tmpl
index 05e453adae..7316333ee6 100644
--- a/template/en/default/email/newchangedmail.txt.tmpl
+++ b/template/en/default/email/newchangedmail.txt.tmpl
@@ -53,7 +53,7 @@ X-Bugzilla-Changed-Fields: [% changedfields.join(" ") %]
 [%- IF comment.count %]
 --- Comment #[% comment.count %] from [% comment.author.identity %] [%+ comment.creation_ts FILTER time(undef, to_user.timezone) %] ---
 [% END %]
-[%+ comment.body_full({ is_bugmail => 1, wrap => 1 }) %]
+[%+ comment.body_full({ is_bugmail => 1, wrap => 1 }) FILTER strip_control_chars %]
 [% END %]
 
 -- [%# Protect the trailing space of the signature marker %]
diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl
index b7a9932e41..7cf75b867c 100644
--- a/template/en/default/request/email.txt.tmpl
+++ b/template/en/default/request/email.txt.tmpl
@@ -81,7 +81,7 @@ Attachment [% attidsummary %]
 [%-# .defined is necessary to avoid a taint issue in Perl < 5.10.1, see bug 509794. %]
 [% IF Bugzilla.cgi.param("comment").defined && Bugzilla.cgi.param("comment").length > 0 %]
 ------- Additional Comments from [% user.identity %]
-[%+ Bugzilla.cgi.param("comment") %]
+[%+ Bugzilla.cgi.param("comment") FILTER strip_control_chars %]
 [% END %]
 
 [%- END %]
-- 
2.47.2