Fix for bug 178841: removes full paths from filenames in attachments table and prevents them from appearing again

r=gerv,bbaetz
a=justdave

2 files changed, 48 insertions, 2 deletions

diff --git a/attachment.cgi b/attachment.cgi
index 971968b3e..33f8c8542 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -276,10 +276,24 @@ sub validateData
   return $data;
 }
 
+my $filename;
 sub validateFilename
 {
   defined $cgi->upload('data')
     || ThrowUserError("file_not_specified");
+
+  $filename = $cgi->upload('data');
+  
+  # Remove path info (if any) from the file name.  The browser should do this
+  # for us, but some are buggy.  This may not work on Mac file names and could
+  # mess up file names with slashes in them, but them's the breaks.  We only
+  # use this as a hint to users downloading attachments anyway, so it's not 
+  # a big deal if it munges incorrectly occasionally.
+  $filename =~ s/^.*[\/\\]//;
+
+  # Truncate the filename to 100 characters, counting from the end of the string
+  # to make sure we keep the filename extension.
+  $filename = substr($filename, -100, 100);
 }
 
 sub validateObsolete
@@ -442,7 +456,7 @@ sub insert
   # Insert a new attachment into the database.
 
   # Escape characters in strings that will be used in SQL statements.
-  my $filename = SqlQuote($cgi->param('data'));
+  $filename = SqlQuote($filename);
   my $description = SqlQuote($::FORM{'description'});
   my $contenttype = SqlQuote($::FORM{'contenttype'});
   my $thedata = SqlQuote($data);
diff --git a/checksetup.pl b/checksetup.pl
index aa91c3a34..cd02538b3 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -1353,7 +1353,7 @@ $table{attachments} =
     description mediumtext not null,
     mimetype mediumtext not null,
     ispatch tinyint,
-    filename mediumtext not null,
+    filename varchar(100) not null,
     thedata longblob not null,
     submitter_id mediumint not null,
     isobsolete tinyint not null default 0, 
@@ -3737,6 +3737,38 @@ if ($sth->rows == 0) {
 }
 
 
+# 2002 November, myk@mozilla.org, bug 178841:
+#
+# Convert the "attachments.filename" column from a ridiculously large
+# "mediumtext" to a much more sensible "varchar(100)".  Also takes
+# the opportunity to remove paths from existing filenames, since they 
+# shouldn't be there for security.  Buggy browsers include them, 
+# and attachment.cgi now takes them out, but old ones need converting.
+#
+{
+    my $ref = GetFieldDef("attachments", "filename");
+    if ($ref->[1] ne 'varchar(100)') {
+        print "Removing paths from filenames in attachments table...\n";
+        
+        $sth = $dbh->prepare("SELECT attach_id, filename FROM attachments " . 
+                             "WHERE INSTR(filename, '/') " . 
+                             "OR INSTR(filename, '\\\\')");
+        $sth->execute;
+        
+        while (my ($attach_id, $filename) = $sth->fetchrow_array) {
+            $filename =~ s/^.*[\/\\]//;
+            my $quoted_filename = $dbh->quote($filename);
+            $dbh->do("UPDATE attachments SET filename = $quoted_filename " . 
+                     "WHERE attach_id = $attach_id");
+        }
+        
+        print "Done.\n";
+        
+        print "Resizing attachments.filename from mediumtext to varchar(100).\n";
+        ChangeFieldType("attachments", "filename", "varchar(100) not null");
+    }
+}
+
 
 #
 # Final checks...