php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

Patch the-magic-patch-fixing-suggestions-from-description for Website problem Bug #54397

Patch version 2011-03-26 20:55 UTC

Return to Bug #54397 | Download this patch
Patch Revisions:

Developer: lekensteyn@gmail.com

Index: common.inc
===================================================================
--- common.inc	(revision 309715)
+++ common.inc	(working copy)
@@ -19,7 +19,7 @@
 <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
  <head>
   <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
-  <title><?php echo $title?></title>
+  <title><?php echo htmlspecialchars($title); ?></title>
   <link rel="stylesheet" href="/style.css" type="text/css" />
  </head>
  <body>
@@ -119,14 +119,14 @@
 function format_author($a, $charset) {
 	$a = recode_header($a, $charset);
 	if (preg_match("/^\s*(.+)\s+\\(\"?(.+?)\"?\\)\s*$/",$a,$ar)) {
-		return "<a href=\"mailto:".htmlspecialchars(urlencode(spam_protect($ar[1])))."\" class=\"email fn n\">".str_replace(" ", "&nbsp;", htmlspecialchars($ar[2]))."</a>";
+		return "<a href=\"mailto:".urlencode(spam_protect($ar[1]))."\" class=\"email fn n\">".str_replace(" ", "&nbsp;", htmlspecialchars($ar[2]))."</a>";
 	}
 	if (preg_match("/^\s*\"?(.+?)\"?\s*<(.+)>\s*$/",$a,$ar)) {
-		return "<a href=\"mailto:".htmlspecialchars(urlencode(spam_protect($ar[2])))."\" class=\"email fn n\">".str_replace(" ", "&nbsp;", htmlspecialchars($ar[1]))."</a>";
+		return "<a href=\"mailto:".urlencode(spam_protect($ar[2]))."\" class=\"email fn n\">".str_replace(" ", "&nbsp;", htmlspecialchars($ar[1]))."</a>";
 	}
 	if (ereg("@",$a)) {
 		$a = spam_protect($a);
-		return "<a href=\"mailto:".htmlspecialchars(urlencode($a))."\" class=\"email fn n\">".htmlspecialchars($a)."</a>";
+		return "<a href=\"mailto:".urlencode($a)."\" class=\"email fn n\">".htmlspecialchars($a)."</a>";
 	}
 	return str_replace(" ", "&nbsp;", htmlspecialchars($a));
 }
@@ -159,6 +159,8 @@
 }
 
 function format_date($d) {
+	/* TODO: handle "invalid" dates like "now" and "yesterday", a valid Date header would look like */
+	/* Date: Mon, 28 Feb 2011 08:49:07 +0000 */
 	$d = strtotime($d);
 	$d = strftime("%c", $d);
 	return str_replace(" ", "&nbsp;", $d);
Index: article.php
===================================================================
--- article.php	(revision 309715)
+++ article.php	(working copy)
@@ -32,9 +32,13 @@
 	error("Failed to get article ". $article);
 }
 
+/* Prevents the poor mail server from suffering if it receives a message with many references */
+/* (References: <xxx> or In-Reply-To: <xxx>) */
+define( 'REFERENCES_LIMIT', 20 );
+
 $started = 0;
 $inheaders = 1; $headers = array();
-$masterheaders = null;
+$masterheaders = array();
 $mimetype = $boundary = $charset = $encoding = "";
 $mimecount = 0; // number of mime parts
 $boundaries = array();
@@ -45,12 +49,18 @@
 	$line = fgets($s);
 	if ($line == ".\r\n") break;
 	if ($inheaders && ($line == "\n" || $line == "\r\n")) {
+		if (empty($masterheaders)) {
+			/* $masterheaders should contain the first headers with From, Subject, */
+			/* etc., not the ones from multiparts */
+			$masterheaders = $headers;
+		}
+
 		$inheaders = 0;
 		if (isset($headers['content-type'])) {
 			if (preg_match('/charset=(["\']?)(\w+)\1/i', $headers['content-type'], $m)) {
 				$charset = trim($m[2]);
 			}
-		
+
 			if(preg_match('/boundary=(["\']?)(.+)\1/is', $headers['content-type'], $m)) {
 				$boundaries[] = trim($m[2]);
 				$boundary = end($boundaries);
@@ -63,7 +73,7 @@
 		}
 		if (!$started) {
 			head("$group: ".format_title($headers['subject'], $charset));
-			start_article($group,$headers,$charset);
+			start_article($group, $masterheaders, $charset);
 			$started = 1;
 		}
 
@@ -95,12 +105,13 @@
 
 			$dl_link = "/getpart.php?group=$group&amp;article=$article&amp;part=$mimecount";
 
-			echo "Attachment: <a href=\"$dl_link\">${link_desc}</a><br />\n";
+			/* Attachment filename and mimetype might contain malicious characters */
+			printf("Attachment: <a href=\"%s\">%s</a><br />\n",
+				$dl_link,
+				htmlspecialchars($link_desc)
+			);
 		}
 
-		if ($masterheaders == null) {
-			$headers = $masterheaders;
-		}
 		continue;
 	}
 	# fix lines that started with a period and got escaped
@@ -108,14 +119,17 @@
 		$line = substr($line,1);
 	}
 
-
 	if ($inheaders) {
-		@list($k,$v) = explode(": ", $line, 2);
-		if ($k && $v) {
-			$headers[strtolower($k)] = $v;
-			$lk = strtolower($k);
+		/* header fields can be split across lines: CRLF WSP where WSP */
+		/* is a space (ASCII 32) or tab (ASCII 9) */
+		if ($lk && ($line[0] == ' ' || $line[0] == "\t")) {
+			$headers[$lk] .= $line;
 		} else {
-			$headers[$lk] .= $line;
+			@list($k,$v) = explode(": ", $line, 2);
+			if ($k && $v) {
+				$headers[strtolower($k)] = $v;
+				$lk = strtolower($k);
+			} // else not a header field
 		}
 	}
 	else {
@@ -131,8 +145,9 @@
 				array_pop($boundaries);
 				$boundary = end($boundaries);
 			} else {
-				# next section; start with an inherited set of headers
-				$headers = $masterheaders;
+				/* Next section: start with no headers, default content type should be
+				/* text/plain, but for now ignore that (see rfc 2046  5.1.3) */
+				$headers = array();
 				$mimetype = "";
 			}
 
@@ -171,7 +186,8 @@
 		# this is some amazingly simplistic code to color quotes/signatures
 		# differently, and turn links into real links. it actually appears
 		# to work fairly well, but could easily be made more sophistimicated.
-		$line = htmlentities($line,ENT_NOQUOTES,"utf-8");
+		/* NOQUOTES? Why? It creates invalid HTML: http:"x */
+		$line = htmlentities($line,ENT_QUOTES,"utf-8");
 		$line = preg_replace("/((mailto|https?|ftp|nntp|news):.+?)(&gt;|\\s|\\)|\\.\\s|$)/","<a href=\"\\1\">\\1</a>\\3",$line);
 		if (!$insig && $line == "-- \r\n") {
 			echo "<span class=\"signature\">";
@@ -189,8 +205,8 @@
 	}
 }
 if ($inheaders && !$started) {
-	head("$group: ". $headers[subject]);
-	start_article($group,$headers,$charset);
+	head("$group: ". $headers['subject']);
+	start_article($group, $masterheaders, $charset);
 }
 if ($insig) {
 	echo "</span>";
@@ -198,7 +214,7 @@
 echo "   </pre>\n";
 echo "  </blockquote>\n";
 
-function start_article ($group,$headers,$charset) {
+function start_article($group, $headers, $charset) {
 	echo "  <blockquote>\n";
 	echo '   <table border="0" cellpadding="2" cellspacing="2" width="100%">' . "\n";
 	# from
@@ -222,21 +238,30 @@
 		echo '     <td class="headervalue">';
 		$r = explode(" ", $ref);
 		$c = 1;
-		$s = nntp_connect(NNTP_HOST)
-		or die("failed to connect to news server");
-		while (list($k,$v) = each($r)) {
-			if (!$v) continue;
-			$v = trim($v);
-			if (!preg_match("/^<.+>\$/", $v)) {
-				continue;
+		/* don't exit the page, just skip references if no connection can be made with the newsserver */
+		if ($s = nntp_connect(NNTP_HOST)) {
+			while (list($k,$v) = each($r)) {
+				if (!$v) continue;
+				$v = trim($v);
+				/* todo: tighten the condition for the message ID, maybe ^<[a-zA-Z0-9.-]@([a-z0-9]+\.)+[a-z]+>$ ? */
+				if (!preg_match("/^<.+>\$/", $v)) {
+					continue;
+				}
+				$res2 = nntp_cmd($s, "XPATH $v",223)
+				or print("failed to get reference article id ".htmlspecialchars($v)."<br />");
+				list(,$v)  = split("/", trim($res2));
+				if (empty($v)) {
+					continue;
+				}
+				/* htmlspecialchars is redundant with urlencode */
+				echo "<a href=\"/$group/".urlencode($v)."\">".($c++)."</a>&nbsp;";
+				/* implement a hard limit to prevent denial of service */
+				/* A better solution might be a reverse search for references */
+				if ($c > REFERENCES_LIMIT) {
+					printf('More than %d references', REFERENCES_LIMIT);
+					break;
+				}
 			}
-			$res2 = nntp_cmd($s, "XPATH $v",223)
-			or print("failed to get reference article id ".htmlspecialchars($v)."<br />");
-			list(,$v)  = split("/", trim($res2));
-			if (empty($v)) {
-				continue;
-			}
-			echo "<a href=\"/$group/".htmlspecialchars(urlencode($v))."\">".($c++)."</a>&nbsp;";
 		}
 		echo "</td>\n";
 	}
@@ -246,7 +271,7 @@
 		echo '     <td class="headervalue">';
 		$r = explode(",", chop($headers["newsgroups"]));
 		while (list($k,$v) = each($r)) {
-			echo "<a href=\"/".htmlspecialchars(urlencode($v))."\">".htmlspecialchars($v)."</a>&nbsp;";
+			echo "<a href=\"/".urlencode($v)."\">".htmlspecialchars($v)."</a>&nbsp;";
 		}
 		echo "</td>\n";
 	}
Index: getpart.php
===================================================================
--- getpart.php	(revision 309715)
+++ getpart.php	(working copy)
@@ -41,8 +41,7 @@
 $emit = false;
 
 $inheaders = 1; $headers = array();
-$masterheaders = null;
-$mimetype = $boundary = $charset = $encoding = "";
+$boundary = $charset = $encoding = "";
 $mimecount = 0; // number of mime parts
 $boundaries = array();
 $lk = '';
@@ -64,7 +63,6 @@
 		}
 		if ($headers['content-type']
 		&& preg_match("/([^;]+)(;|\$)/", $headers['content-type'], $m)) {
-			$mimetype = trim(strtolower($m[1]));
 			++$mimecount;
 		}
 
@@ -72,20 +70,26 @@
 
 		$encoding = strtolower(trim($headers['content-transfer-encoding']));
 		if ($emit) {
-			if (isset($headers['content-type'])) {
-				header('Content-Type: ' . $headers['content-type']);
+			/* check if content-type exist is made above */
+			header('Content-Type: ' . $headers['content-type']);
+			/* Do not rely on user-provided content-deposition header, generate own one to */
+			/* make the content downloadable, do NOT use inline, we can't trust the attachment*/
+			/* Downside of this approach: images should be downloaded before use */
+			/* this is safer though, and prevents doing evil things on php.net domain */
+			$contentdisposition = 'attachment';
+			if (isset($headers['content-disposition'])
+			&& preg_match('/filename=([\'"]?).+?\1/', $headers['content-disposition'], $m)) {
+				$contentdisposition .= '; ' . $m[0];
 			}
-			if (isset($headers['content-disposition'])) {
-				header('Content-Disposition: ' . $headers['content-disposition']);
-			}
+			header('Content-Disposition: ' . $contentdisposition);
+			// if (isset($headers['content-disposition'])) {
+			//	header('Content-Disposition: ' . $headers['content-disposition']);
+			//}
 			if (isset($headers['content-description'])) {
 				header('Content-Description: ' . $headers['content-description']);
 			}
 		}
 
-		if ($masterheaders == null) {
-			$headers = $masterheaders;
-		}
 		continue;
 	}
 	# fix lines that started with a period and got escaped
@@ -93,12 +97,16 @@
 		$line = substr($line,1);
 	}
 	if ($inheaders) {
-		list($k,$v) = explode(": ", $line, 2);
-		if ($k && $v) {
-			$headers[strtolower($k)] = $v;
-			$lk = strtolower($k);
+		/* header fields can be split across lines: CRLF WSP where WSP */
+		/* is a space (ASCII 32) or tab (ASCII 9) */
+		if ($lk && ($line[0] == ' ' || $line[0] == "\t")) {
+			$headers[$lk] .= $line;
 		} else {
-			$headers[$lk] .= $line;
+			@list($k,$v) = explode(": ", $line, 2);
+			if ($k && $v) {
+				$headers[strtolower($k)] = $v;
+				$lk = strtolower($k);
+			} // else not a header field
 		}
 	} else {
 
@@ -113,9 +121,8 @@
 				array_pop($boundaries);
 				$boundary = end($boundaries);
 			} else {
-				# next section; start with an inherited set of headers
-				$headers = $masterheaders;
-				$mimetype = "";
+				/* next section starts with no headers */
+				$headers = null;
 			}
 
 			continue;
@@ -136,6 +143,10 @@
 
 		echo $line;
 
+		/* done with attachment, no need to continue */
+		if ($emit) {
+			break;
+		}
 	}
 }
 
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 01:01:28 2024 UTC