Browse Source

fix several potential issues in handling comment files

El RIDO 21 hours ago
parent
commit
d853069a12
2 changed files with 42 additions and 25 deletions
  1. 1 0
      CHANGELOG.md
  2. 41 25
      lib/Data/Filesystem.php

+ 1 - 0
CHANGELOG.md

@@ -10,6 +10,7 @@
 * CHANGED: Removed support for Bootstrap 3 as it requires jQuery.
 * CHANGED: Removed support for Bootstrap 3 as it requires jQuery.
 * FIXED: Prevent browsers guessing MIME types on JSON(LD) API responses (#164)
 * FIXED: Prevent browsers guessing MIME types on JSON(LD) API responses (#164)
 * FIXED: Insert quoted base path into JSON-LD documents
 * FIXED: Insert quoted base path into JSON-LD documents
+* FIXED: Several potential issues in handling comment files (#1901)
 
 
 ## 2.0.6 (2026-08-08)
 ## 2.0.6 (2026-08-08)
 * CHANGED: Stricter MIME type validation, divergent files get no preview and forced download link
 * CHANGED: Stricter MIME type validation, divergent files get no preview and forced download link

+ 41 - 25
lib/Data/Filesystem.php

@@ -23,6 +23,23 @@ use PrivateBin\Json;
  */
  */
 class Filesystem extends AbstractData
 class Filesystem extends AbstractData
 {
 {
+    /**
+     * line in generated .htaccess files, to protect exposed directories from being browsable on apache web servers
+     *
+     * @const string
+     */
+    const HTACCESS_LINE = 'Require all denied';
+
+    /**
+     * ID glob() pattern of valid document IDs as well as parent and comment IDs
+     * 16 lower case characters for 8 bytes in hexadecimal encoding
+     *
+     * @const string
+     */
+    const ID_PATTERN =
+        '[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]' .
+        '[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]';
+
     /**
     /**
      * glob() pattern of the two folder levels and the paste files under the
      * glob() pattern of the two folder levels and the paste files under the
      * configured path. Needs to return both files with and without .php suffix,
      * configured path. Needs to return both files with and without .php suffix,
@@ -36,8 +53,7 @@ class Filesystem extends AbstractData
      */
      */
     const PASTE_FILE_PATTERN = DIRECTORY_SEPARATOR . '[a-f0-9][a-f0-9]' .
     const PASTE_FILE_PATTERN = DIRECTORY_SEPARATOR . '[a-f0-9][a-f0-9]' .
         DIRECTORY_SEPARATOR . '[a-f0-9][a-f0-9]' . DIRECTORY_SEPARATOR .
         DIRECTORY_SEPARATOR . '[a-f0-9][a-f0-9]' . DIRECTORY_SEPARATOR .
-        '[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]' .
-        '[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]*';
+        self::ID_PATTERN . '*';
 
 
     /**
     /**
      * first line in paste or comment files, to protect their contents from browsing exposed data directories
      * first line in paste or comment files, to protect their contents from browsing exposed data directories
@@ -46,13 +62,6 @@ class Filesystem extends AbstractData
      */
      */
     const PROTECTION_LINE = '<?php http_response_code(403); /*';
     const PROTECTION_LINE = '<?php http_response_code(403); /*';
 
 
-    /**
-     * line in generated .htaccess files, to protect exposed directories from being browsable on apache web servers
-     *
-     * @const string
-     */
-    const HTACCESS_LINE = 'Require all denied';
-
     /**
     /**
      * path in which to persist something
      * path in which to persist something
      *
      *
@@ -163,8 +172,10 @@ class Filesystem extends AbstractData
             // convert comments, too
             // convert comments, too
             $discdir = $this->_dataid2discussionpath($pasteid);
             $discdir = $this->_dataid2discussionpath($pasteid);
             if (is_dir($discdir)) {
             if (is_dir($discdir)) {
-                foreach (new DirectoryIterator($discdir) as $file) {
-                    if ($file->getExtension() !== 'php' && strlen($file->getFilename()) >= 16) {
+                foreach (new GlobIterator($discdir . DIRECTORY_SEPARATOR . $pasteid .
+                    '.' . self::ID_PATTERN . '.' . self::ID_PATTERN) as $file
+                ) {
+                    if ($file->isFile()) {
                         $this->_prependRename($file->getPathname(), $file->getPathname() . '.php');
                         $this->_prependRename($file->getPathname(), $file->getPathname() . '.php');
                     }
                     }
                 }
                 }
@@ -208,22 +219,14 @@ class Filesystem extends AbstractData
         $comments = [];
         $comments = [];
         $discdir  = $this->_dataid2discussionpath($pasteid);
         $discdir  = $this->_dataid2discussionpath($pasteid);
         if (is_dir($discdir)) {
         if (is_dir($discdir)) {
-            foreach (new DirectoryIterator($discdir) as $file) {
+            foreach (new GlobIterator($discdir . DIRECTORY_SEPARATOR . $pasteid .
+                '.' . self::ID_PATTERN . '.' . self::ID_PATTERN . '.php') as $file
+            ) {
                 // Filename is in the form pasteid.commentid.parentid.php:
                 // Filename is in the form pasteid.commentid.parentid.php:
                 // - pasteid is the paste this reply belongs to.
                 // - pasteid is the paste this reply belongs to.
                 // - commentid is the comment identifier itself.
                 // - commentid is the comment identifier itself.
                 // - parentid is the comment this comment replies to (It can be pasteid)
                 // - parentid is the comment this comment replies to (It can be pasteid)
                 if ($file->isFile()) {
                 if ($file->isFile()) {
-                    $items = explode('.', $file->getBasename('.php'));
-                    if (
-                        $file->getExtension() !== 'php' ||
-                        count($items) !== 3 ||
-                        $items[0] !== $pasteid ||
-                        preg_match('/\A[a-f0-9]{16}\z/', $items[1]) !== 1 ||
-                        preg_match('/\A[a-f0-9]{16}\z/', $items[2]) !== 1
-                    ) {
-                        continue;
-                    }
                     $comment = $this->_get($file->getPathname());
                     $comment = $this->_get($file->getPathname());
                     if (
                     if (
                         !is_array($comment) ||
                         !is_array($comment) ||
@@ -232,6 +235,7 @@ class Filesystem extends AbstractData
                     ) {
                     ) {
                         continue;
                         continue;
                     }
                     }
+                    $items = explode('.', $file->getBasename('.php'));
                     // Add some meta information not contained in file.
                     // Add some meta information not contained in file.
                     $comment['id']       = $items[1];
                     $comment['id']       = $items[1];
                     $comment['parentid'] = $items[2];
                     $comment['parentid'] = $items[2];
@@ -529,11 +533,23 @@ class Filesystem extends AbstractData
     {
     {
         // don't overwrite already converted file
         // don't overwrite already converted file
         if (!is_readable($destFile)) {
         if (!is_readable($destFile)) {
-            $handle = fopen($srcFile, 'r', false, stream_context_create());
-            file_put_contents($destFile, self::PROTECTION_LINE . PHP_EOL);
-            file_put_contents($destFile, $handle, FILE_APPEND);
+            $handle = @fopen($srcFile, 'r', false, stream_context_create());
+            if ($handle === false) {
+                error_log('Error reading to be converted document: ' . $srcFile);
+                return;
+            }
+            $written = @file_put_contents($destFile, self::PROTECTION_LINE . PHP_EOL);
+            if ($written !== false) {
+                $written = @file_put_contents($destFile, $handle, FILE_APPEND);
+            }
+            if ($written === false) {
+                error_log('Error writing converted document: ' . $destFile);
+            }
             fclose($handle);
             fclose($handle);
             chmod($destFile, 0640); // protect file from access by other users on the host
             chmod($destFile, 0640); // protect file from access by other users on the host
+            if ($written === false) {
+                return;
+            }
         }
         }
         if (!unlink($srcFile)) {
         if (!unlink($srcFile)) {
             error_log('Error deleting converted document: ' . $srcFile);
             error_log('Error deleting converted document: ' . $srcFile);