Bug#713001: Lack of proper escaping in http://www.debian.org/mirror/submit [Re: Vulnerability]

[Don Armstrong <don@debian.org>]

Control: tag -1 patch

On Fri, 21 Jun 2013, codie manjot wrote:
> POC - - Open the above given vulnerable link - Once opened, copy the
> below given xss script in all the fields on that webpage & then
> click on submit. the malicious javascript was successfully injected
> on the webpage.

The attached patch fixes this problem.

As a side note, could we please put the code for the scripts running on
cgi.debian.org into a publicly accessible VCS repository (ideally git)
on git.debian.org or similar?


-- 
Don Armstrong                      http://www.donarmstrong.com

<Clint> why the hell does kernel-source-2.6.3 depend on xfree86-common?
<infinity> It... Doesn't?
<Clint> good point

--- submit_mirror.pl.orig	2013-06-21 14:20:13.000000000 -0700
+++ submit_mirror.pl	2013-06-21 14:31:53.000000000 -0700
@@ -5,6 +5,20 @@
 
 # used by www.d.o/mirror/submit
 
+use HTML::Entities;
+
+# encode html entities appropriately; if given an array in list
+# context, return the array; otherwise return the concatenation of
+# everything given
+sub html_escape {
+    my @r = map {HTML::Entities::encode_entities($_)} @_;
+    if (wantarray) {
+        return @r;
+    } else {
+        return join('',@r);
+    }
+}
+
 require 5.001;
 
 my $public_dest = 'submit@bugs.debian.org';
@@ -35,7 +49,7 @@
 if ($site =~ /^([\w.-]+)$/) {
   $site = $1; # now untainted
 } else {
-  print "<p>Broken data given as site name: ".$query->param('site')."\n";
+  print "<p>Broken data given as site name: ".html_escape($query->param('site'))."\n";
   print "<p>Entry not submitted!";
   exit;
 }
@@ -72,7 +86,7 @@
           if ($query->param($type) =~ /^[\/\w-]+$/) {
             $mirror_types{$type} = $query->param($type);
           } else {
-            print "Broken data given: ".$query->param($type)."\n";
+            print "Broken data given: ".html_escape($query->param($type))."\n";
             print "Entry not submitted!";
             exit;
           }
@@ -153,7 +167,7 @@
 my $msg;
 if (defined($submissiontype) && $submissiontype =~ /^(new|update)$/) {
    $msg .= "Submission-Type: $submissiontype\n";
-   print "<p>Submission-Type: $submissiontype</p>\n";
+   print "<p>Submission-Type: ".html_escape($submissiontype)."</p>\n";
 } else {
    print "<p>Submission type not given.\n";
    print "<p>Entry not submitted!";