www/132344: [patch] www/en/cgi/query-pr.cgi broken base64 attachments

[David Horn]

>Number:         132344
>Category:       www
>Synopsis:       [patch] www/en/cgi/query-pr.cgi broken base64 attachments
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-www
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Mar 05 21:10:01 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     David Horn
>Release:        7.1
>Organization:
>Environment:
n/a
>Description:
The query-pr.cgi script interface to gnats tries very hard to decode and display attachments properly.

www/en/cgi/query-pr.cgi

Unfortunately, MIME headers do not always play nice, and there are many instances of inproper display of base64 encoded attachments.

The real issue is that if either "X-Attachment-Id:" or "Content-Disposition:" headers occur after "Content-Transfer-Encoding" headers, then decode_base64() fails, and displays unprintable characters.

example pr's that do not display base64 attachments properly:

ports/130991
ports/130144
ports/130000
>How-To-Repeat:
Go to http://www.freebsd.org/cgi/query-pr.cgi?pr=130144 in your web browser, and notice that the attachment displays unprintable characters.

Repeat for 130144 and 130000 as slightly different examples.
>Fix:
-Parse out the X-Attachment-Id header if found
-do not assume that content-transfer-encoding is the last header
-Check for nonprintables on result of decode_base64()

This fix seems to work for all broken base64 attachment pr's that I have found.  To test the check for nonprintables, just mangle one of the prs by adding "X-Broken-Header: asdfasdf" to the end of the mime header just before the base64 payload starts, and the code will break again, but at least will be caught by the :isascii: check for nonprintables

Unified diff attached.

Patch attached with submission follows:

--- query-pr.cgi.original	2009-03-05 15:26:02.000000000 -0500
+++ query-pr.cgi	2009-03-05 15:47:03.000000000 -0500
@@ -610,7 +610,13 @@
 								if ($patchname =~ /$binary_filetypes/) {
 									$outp = "(Binary attachment not viewable.)\n";
 								} else {
+									$outp =~ s/^X-Attachment-Id: \x0a?//i;
+									$outp =~ s/^f_.{7}[a-z]?[0-9]*//i;
+									$outp =~ s/^file[0-9]*//i;
 									$outp = decode_base64($outp);
+									if ($outp =~ /[[:^ascii:]]/) { 
+											$outp = "(Unable to decode attachment using base64.)\n";
+									}
 								}
 
 								$outp = "--- $patchname begins here ---\n"
@@ -638,7 +644,9 @@
 							}
 							next;
 						} else {
-							$mime_endheader = 1;
+							# XXX: Commented out to fix an issue if transfer-encoding is not the 
+							#	   last header before base64 payload really starts.
+							#$mime_endheader = 1;
 							if ($mime_headers{'transfer-encoding'}) {
 								my $enc = $mime_headers{'transfer-encoding'};
 								if ($enc =~ /^\s*["']?base64["']?\s*$/i) {
@@ -1004,7 +1012,14 @@
 		$inpatch = 0;
 		$mime_boundary = undef;
 		if ($outp ne "") {
-			print decode_base64($outp);
+			$outp =~ s/^X-Attachment-Id: \x0a?//i;
+			$outp =~ s/^f_.{7}[a-z]?[0-9]*//i;
+			$outp =~ s/^file[0-9]*//i;
+			my $decoded_file = decode_base64($outp);
+			if ($decoded_file =~ /[[:^ascii:]]/) { 
+				$decoded_file = $outp;
+			}
+			print ($decoded_file);
 			$outp = "";
 		}
 		return -1;


>Release-Note:
>Audit-Trail:
>Unformatted: