[PHP-WEBMASTER] [web-news] master: Apply additional output hardening

[email protected] (NickSdot via Derick Rethans)
Newsgroups php.webmaster
Message-ID <[email protected]>
Author: NickSdot (NickSdot)
Committer: Derick Rethans (derickr)
Date: 2026-08-10T16:42:10+01:00

Commit: https://github.com/php/web-news/commit/9c2d21ac3f47bec2cff7685e760b93b88da86f72
Raw diff: https://github.com/php/web-news/commit/9c2d21ac3f47bec2cff7685e760b93b88da86f72.diff

Apply additional output hardening

Changed paths:
  M  article.php
  M  group.php
  M  index.php
  M  lib/ThreadTree.php
  M  lib/common.php
  M  lib/group-navbar.php


Diff:

diff --git a/article.php b/article.php
index 7db4dbd..a9d5760 100644
--- a/article.php
+++ b/article.php
@@ -60,14 +60,17 @@
 }
 
 head("{$group}: " . format_title($mail['headers']['subject'], 'utf-8'));
+$cleanGroupUrl = urlencode($group);
+$cleanGroup = clean($group);
+
 echo '<nav class="secondary-nav">';
 echo ' <ul class="breadcrumbs">';
 echo '  <li class="breadcrumbs-item"><a class="breadcrumbs-item-link" href="/">PHP Mailing Lists</a></li>';
 echo '  <li class="breadcrumbs-item"><a class="breadcrumbs-item-link" href="/' .
-    htmlspecialchars($group, ENT_QUOTES, "UTF-8") . '">' .
-    htmlspecialchars($group, ENT_QUOTES, "UTF-8") . '</a></li>';
+    $cleanGroupUrl . '">' .
+    $cleanGroup . '</a></li>';
 echo '  <li class="breadcrumbs-item"><a class="breadcrumbs-item-link" href="/' .
-    htmlspecialchars($group, ENT_QUOTES, "UTF-8") . '/' . $article . '">' .
+    $cleanGroupUrl . '/' . $article . '">' .
     format_title($mail['headers']['subject'], 'utf-8') . '</a></li>';
 echo ' </ul>';
 echo '</nav>';
@@ -107,7 +110,7 @@
     echo '     <td class="headervalue" ' . (empty($refsResolved) ? 'colspan="3"' : null) . '>';
     $r = explode(",", rtrim($mail['headers']['newsgroups']));
     foreach ($r as $v) {
-        echo "<a href=\"/" . urlencode($v) . "\">" . htmlspecialchars($v) . "</a>&nbsp;";
+        echo "<a href=\"/" . urlencode($v) . "\">" . clean($v) . "</a>&nbsp;";
     }
     echo "</td>\n";
 }
@@ -331,7 +334,7 @@
         $name = $attachment['filename'];
 
         if ($mimetype == 'text/plain') {
-            echo htmlspecialchars($attachment['data']);
+            echo clean($attachment['data']);
             continue;
         }
 
@@ -347,14 +350,14 @@
             $link_desc .= " " . $description;
         }
 
-        $dl_link = "/getpart.php?group=$group&amp;article=$article&amp;part=$mimecount";
-        $link_desc = htmlspecialchars($link_desc, ENT_QUOTES, 'UTF-8');
+        $dl_link = "/getpart.php?group=" . urlencode($group) . "&amp;article=" . urlencode((string) $article) . "&amp;part=" . urlencode((string) $mimecount);
+        $link_desc = clean($link_desc);
 
         /* Attachment filename and mimetype might contain malicious characters */
         printf(
             'Attachment: <a href="%s">%s</a><br />' . "\n",
             $dl_link,
-            htmlspecialchars($link_desc)
+            $link_desc,
         );
     }
 }
@@ -380,7 +383,6 @@
 
 // Does not check existence of next, so consider this the super duper fast [broken] version
 // Based off navbar() in group.php
-$group = htmlspecialchars($group, ENT_QUOTES, "UTF-8");
 $current = $article;
 
 echo '  <table class="standard">' . "\n";
@@ -388,15 +390,15 @@
 echo '    <th class="nav">';
 
 if ($current > 1) {
-    echo '     <a href="/' , $group , '/' , ($current - 1) , '"><b>&laquo; <span>previous</span></b></a>';
+    echo '     <a href="/' , $cleanGroupUrl , '/' , ($current - 1) , '"><b>&laquo; <span>previous</span></b></a>';
 } else {
     echo '&nbsp;';
 }
 
 echo '    </th>' . "\n";
-echo '    <th class="align-center">' . "$group (#$current)</th>\n";
+echo '    <th class="align-center">' . $cleanGroup . " (#$current)</th>\n";
 echo '    <th class="nav align-right">';
-echo '     <a href="/' , $group , '/' , ($current + 1) , '"><b><span>next</span> &raquo;</b></a>';
+echo '     <a href="/' , $cleanGroupUrl , '/' , ($current + 1) , '"><b><span>next</span> &raquo;</b></a>';
 echo '    </th>' . "\n";
 echo '   </tr>' . "\n";
 echo '  </table>' . "\n";
diff --git a/group.php b/group.php
index 005941e..fee98b1 100644
--- a/group.php
+++ b/group.php
@@ -33,13 +33,14 @@
 $baseUrlParts = parse_url($NEWS_WEB_BASE_URL);
 $cleanBaseHost = clean($baseUrlParts['host'] . (isset($baseUrlParts['port']) ? ':' . $baseUrlParts['port'] : ''));
 $cleanGroupUrl = urlencode($group);
+$cleanGroupHtml = clean($group);
 switch ($format) {
     case 'rss':
         header("Content-type: text/xml");
         echo '<?xml version="1.0" encoding="utf-8"?>' . "\n";?>
 <rss version="2.0">
  <channel> 
-  <title><?php echo $cleanBaseHost; ?>: <?php echo $group?></title>
+  <title><?php echo $cleanBaseHost; ?>: <?php echo $cleanGroupHtml?></title>
   <link><?php echo $cleanBaseUrl; ?>/group.php?group=<?php echo $cleanGroupUrl?></link>
   <description></description>
         <?php
@@ -52,9 +53,9 @@
         xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns="http://my.netscape.com/rdf/simple/0.9/">
  <channel>
-  <title><?php echo $cleanBaseHost; ?>: <?php echo $group?></title>
+  <title><?php echo $cleanBaseHost; ?>: <?php echo $cleanGroupHtml?></title>
   <link><?php echo $cleanBaseUrl; ?>/group.php?group=<?php echo $cleanGroupUrl?></link>
-  <description><?php echo $group?> Newsgroup at <?php echo $NNTP_HOST; ?></description>
+  <description><?php echo $cleanGroupHtml?> Newsgroup at <?php echo clean($NNTP_HOST); ?></description>
   <language>en-US</language>
  </channel>
         <?php
@@ -66,12 +67,12 @@
         echo ' <ul class="breadcrumbs">';
         echo '  <li class="breadcrumbs-item"><a class="breadcrumbs-item-link" href="/">PHP Mailing Lists</a></li>';
         echo '  <li class="breadcrumbs-item"><a class="breadcrumbs-item-link" href="/',
-            htmlspecialchars($group, ENT_QUOTES, "UTF-8") . '">',
-            htmlspecialchars($group, ENT_QUOTES, "UTF-8") . '</a></li>';
+            $cleanGroupUrl . '">',
+            $cleanGroupHtml . '</a></li>';
         echo ' </ul>';
         echo '</nav>';
         echo '<section class="content">';
-        echo '<h1>' . htmlspecialchars($group, ENT_QUOTES, "UTF-8") . '</h1>';
+        echo '<h1>' . $cleanGroupHtml . '</h1>';
         if ($i == 0) {
             /* Special header of info for the main page for a group */
             $groups = $nntpClient->listGroups($group);
@@ -92,7 +93,7 @@
                         </p>
                     <?php
                 }
-                $subscription_address = htmlspecialchars(get_subscribe_address($group));
+                $subscription_address = clean(get_subscribe_address($group));
                 ?>
                     <form class="subscription-form" method="POST" action="/subscribe.php">
                         <input type="hidden" name="group" value="<?= clean($group) ?>">
@@ -158,7 +159,7 @@
             echo "   <link>$cleanArticleLink</link>\n";
             echo "   <title>", format_subject($details['subject'], $charset), "</title>\n";
             echo "   <description>",
-                htmlspecialchars(format_author($details['author'], $charset), ENT_QUOTES, "UTF-8"),
+                clean(format_author($details['author'], $charset)),
                 "</description>\n";
             echo "   <pubDate>$date822</pubDate>\n";
             echo "  </item>\n";
@@ -168,7 +169,7 @@
             echo "  <title>", format_subject($details['subject'], $charset), "</title>\n";
             echo "  <link>$cleanArticleLink</link>\n";
             echo "  <description>",
-                htmlspecialchars(format_author($details['author'], $charset), ENT_QUOTES, "UTF-8"),
+                clean(format_author($details['author'], $charset)),
                 "</description>\n";
             echo "  <pubDate>$date822</pubDate>\n";
             echo " </item>\n";
@@ -176,14 +177,14 @@
         case 'html':
         default:
             echo "   <tr>\n";
-            echo "    <td><a href=\"/$group/$articleNumber\">$articleNumber</a></td>\n";
-            echo "    <td><a href=\"/$group/$articleNumber\">";
+            echo "    <td><a href=\"$cleanArticlePath\">" . clean($articleNumber) . "</a></td>\n";
+            echo "    <td><a href=\"$cleanArticlePath\">";
             echo format_subject($details['subject'], $charset);
             echo "</a></td>\n";
             echo "    <td class=\"vcard\">" . format_author($details['author'], $charset) . "</td>\n";
             echo "    <td class=\"align-center\"><span class='monospace mod-small'>" .
                 format_date($details['date']) . "</span></td>\n";
-            echo "    <td class=\"align-right\">{$details['lines']}</td>\n";
+            echo "    <td class=\"align-right\">" . clean($details['lines']) . "</td>\n";
             echo "   </tr>\n";
     }
 }
diff --git a/index.php b/index.php
index 20bd3e2..c6008be 100644
--- a/index.php
+++ b/index.php
@@ -17,7 +17,7 @@
 
 head();
 
-$DISPLAY_NNTP_HOST = htmlspecialchars(($NNTP_HOST == 'localhost') ? 'news-web.php.net' : $NNTP_HOST);
+$CLEAN_DISPLAY_NNTP_HOST = clean(($NNTP_HOST == 'localhost') ? 'news-web.php.net' : $NNTP_HOST);
 ?>
 
 <nav class="secondary-nav">
@@ -33,7 +33,7 @@
    <p>
     The PHP project collaborates across a number of mailing lists. The archives
     are available through this site and via NNTP at
-    <a href="news://<?= $DISPLAY_NNTP_HOST ?>"> <?= $DISPLAY_NNTP_HOST ?></a>.
+    <a href="news://<?= $CLEAN_DISPLAY_NNTP_HOST ?>"> <?= $CLEAN_DISPLAY_NNTP_HOST ?></a>.
    </p>
    <p>
     Instructions for subscribing to active lists by email can be found on the page
@@ -61,13 +61,14 @@
             $last_status == 'y' ? 'Discussion Lists' : 'Inactive Lists',
             "</th></tr>\n";
     }
+    $cleanGroupUrl = urlencode($group);
     echo "       <tr>\n";
-    echo "        <td><a class=\"active{$details['status']}\" href=\"/$group\">$group</a></td>\n";
-    echo "        <td>", htmlspecialchars($descriptions[$group]), "</td>\n";
+    echo "        <td><a class=\"active" . clean($details['status']) . "\" href=\"/$cleanGroupUrl\">" . clean($group) . "</a></td>\n";
+    echo "        <td>", clean($descriptions[$group]), "</td>\n";
     echo "        <td class=\"align-right\">", $details['high'] - $details['low'] + 1, "</td>\n";
     echo "        <td class=\"align-center\">";
     if ($details['status'] != 'n') {
-        echo "<a href=\"group.php?group=$group&amp;format=rss\">RSS</a>";
+        echo "<a href=\"group.php?group=$cleanGroupUrl&amp;format=rss\">RSS</a>";
     }
     echo "</td>\n";
     echo "       </tr>\n";
diff --git a/lib/ThreadTree.php b/lib/ThreadTree.php
index 399cac5..bf1bb7f 100644
--- a/lib/ThreadTree.php
+++ b/lib/ThreadTree.php
@@ -60,12 +60,13 @@ protected function printArticleAndChildren($messageId, $group, $charset, $depth
             #unset($this->articleNumbers[$messageId]);
 
             $details = $this->articles[$articleNumber];
+            $cleanArticlePath = "/" . urlencode($group) . "/" . urlencode((string) $articleNumber);
 
             echo "   <tr>\n";
-            echo "    <td align=\"center\"><a href=\"/$group/$articleNumber\">$articleNumber</a></td>\n";
+            echo "    <td align=\"center\"><a href=\"$cleanArticlePath\">" . clean($articleNumber) . "</a></td>\n";
             echo "    <td>";
             echo str_repeat("&nbsp; &nbsp;", $depth ?? 0);
-            echo "<a href=\"/$group/$articleNumber\">";
+            echo "<a href=\"$cleanArticlePath\">";
             echo format_subject($details['subject'], $charset);
             echo "</a></td>\n";
             echo "    <td class=\"vcard\">" . format_author($details['author'], $charset) . "</td>\n";
@@ -144,7 +145,7 @@ public function printThread(
             $details = $this->articles[$articleNumber];
 
             if ($articleNumber != $activeArticleNumber) {
-                echo "<a href=\"/$group/$articleNumber\">";
+                echo "<a href=\"/" . urlencode($group) . "/" . urlencode((string) $articleNumber) . "\">";
             } else {
                 echo "<b>";
             }
diff --git a/lib/common.php b/lib/common.php
index 46244c4..5dd2ac5 100644
--- a/lib/common.php
+++ b/lib/common.php
@@ -8,15 +8,15 @@ function error($str)
 {
     head("PHP news : error");
     echo "<section class=\"content\"><blockquote><strong>Error:</strong> ",
-       to_utf8($str), "</blockquote></section>\n";
+       clean(to_utf8($str)), "</blockquote></section>\n";
     foot();
     die();
 }
 
 /* Borrowed from web-php repo. */
-function clean($var)
+function clean($var): string
 {
-    return htmlspecialchars($var, \ENT_QUOTES);
+    return htmlspecialchars((string) $var, ENT_QUOTES | ENT_SUBSTITUTE, "UTF-8");
 }
 
 // Try to check that this email address is valid
@@ -112,7 +112,7 @@ function head($title = "PHP Mailing Lists (PHP News)")
  <head>
   <meta charset="utf-8">
   <meta name="viewport" content="width=device-width, initial-scale=1.0">
-  <title><?php echo htmlspecialchars($title); ?></title>
+  <title><?php echo clean($title); ?></title>
   <link href="//fonts.googleapis.com/css?family=Fira+Sans|Source+Sans+Pro:300,400,600,400italic,600italic|Source+Code+Pro&amp;subset=latin,latin-ext" rel="stylesheet">
   <link rel="stylesheet" href="/style.css" type="text/css" />
   <link rel="shortcut icon" href="//www.php.net/favicon.ico">
@@ -246,7 +246,11 @@ function format_author($a, $charset = 'iso-8859-1', $nameOnly = false)
         $email= spam_protect($ar[1]);
         $name = $ar[2];
     }
-    elseif (preg_match("/^\s*\"?(.+?)\"?\s*<(.+)>\s*$/", $a, $ar)) {
+
+    // Treat the address as one angle-bracketed part, so extra
+    // brackets in the display name do not confuse parsing.
+
+    elseif (preg_match("/^\s*\"?(.+?)\"?\s*<([^<>]+)>\s*$/", $a, $ar)) {
         $email = spam_protect($ar[2]);
         $name = $ar[1];
     }
@@ -255,13 +259,14 @@ function format_author($a, $charset = 'iso-8859-1', $nameOnly = false)
     } else {
         $email = $name = $a;
     }
+
     $name = clean($name);
 
     if ($nameOnly) {
         return str_replace(" ", "&nbsp;", $name);
     } else {
         return "<a href=\"mailto:" .
-            htmlspecialchars(urlencode($email), ENT_QUOTES, "UTF-8") .
+            clean(urlencode($email)) .
             "\" class=\"email fn n\">" .
             str_replace(" ", "&nbsp;", $name) . "</a>";
     }
@@ -281,7 +286,7 @@ function format_subject($s, $charset = 'iso-8859-1', $trimRe = false)
     } else {
         $s = wordwrap($s, 150);
     }
-    return nl2br(htmlspecialchars($s, ENT_QUOTES, "UTF-8"));
+    return nl2br(clean($s));
 }
 
 
@@ -296,7 +301,7 @@ function format_title($s, $charset = 'iso-8859-1', $trimRe = false)
     } else {
         $s = wordwrap($s, 150);
     }
-    return htmlspecialchars($s, ENT_QUOTES, "UTF-8");
+    return clean($s);
 }
 
 function format_date($d, $format = 'r')
diff --git a/lib/group-navbar.php b/lib/group-navbar.php
index e8d92bc..cfaad43 100644
--- a/lib/group-navbar.php
+++ b/lib/group-navbar.php
@@ -7,7 +7,7 @@ function navbar($g, $f, $l, $i)
     echo '    <th class="nav">';
     if ($i > $f) {
         $p = max($i - 20, $f);
-        echo "<a href=\"/" . htmlspecialchars($g, ENT_QUOTES, "UTF-8") . "/start/$p\">",
+        echo "<a href=\"/" . urlencode($g) . "/start/$p\">",
             "<b>&laquo; <span>previous</span></b></a>";
     } else {
         echo "&nbsp;";
@@ -15,11 +15,11 @@ function navbar($g, $f, $l, $i)
     echo '</th>' . "\n";
     $j = min($i + 20, $l);
     $c = $l - $f + 1;
-    echo '    <th class="align-center">' . htmlspecialchars($g, ENT_QUOTES, "UTF-8") . " ($i-$j of $c)</th>\n";
+    echo '    <th class="align-center">' . clean($g) . " ($i-$j of $c)</th>\n";
     echo '    <th class="nav align-right">';
     if ($i + 20 <= $l) {
         $n = min($i + 20, $l - 19);
-        echo "<a href=\"/", htmlspecialchars($g, ENT_QUOTES, "UTF-8") . "/start/$n\">",
+        echo "<a href=\"/", urlencode($g) . "/start/$n\">",
             "<b><span>next</span> &raquo;</b></a>";
     } else {
         echo "&nbsp;";
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.