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(" ", " ", htmlspecialchars($ar[2]))."</a>";
+ return "<a href=\"mailto:".urlencode(spam_protect($ar[1]))."\" class=\"email fn n\">".str_replace(" ", " ", 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(" ", " ", htmlspecialchars($ar[1]))."</a>";
+ return "<a href=\"mailto:".urlencode(spam_protect($ar[2]))."\" class=\"email fn n\">".str_replace(" ", " ", 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(" ", " ", 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(" ", " ", $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&article=$article&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):.+?)(>|\\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> ";
+ /* 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> ";
}
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> ";
+ echo "<a href=\"/".urlencode($v)."\">".htmlspecialchars($v)."</a> ";
}
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;
+ }
}
}
|