Просмотр исходного кода

Merge branch 'master' into authenticate-comment-parents

El RIDO 1 неделя назад
Родитель
Сommit
6824f6342f

+ 1 - 1
.devcontainer/devcontainer.json

@@ -38,7 +38,7 @@
     "features": {
         "ghcr.io/devcontainers-extra/features/mocha:2": {},
         "ghcr.io/yassinedoghri/devcontainers/php-extensions-installer:1": {
-            "extensions": "gd"
+            "extensions": "gd intl"
         }
     },
     "forwardPorts": [

+ 9 - 6
CHANGELOG.md

@@ -1,21 +1,24 @@
 # PrivateBin version history
 
 ## 2.1.1 (not yet released)
-* ADDED: Comments authenticate the IDs of their document and parent comment by default. Older comments without these IDs are still accepted and shown, unless the new `discussionrequireids` option is enabled
-* FIXED: Creating a document failed with a JavaScript error when the `discussion`, `password`, e-mail or QR code option is disabled, as the top navigation accessed the omitted elements
 * ADDED: Translation for Korean (#2071)
+* ADDED: Comments authenticate the IDs of their document and parent comment by default. Older comments without these IDs are still accepted and shown, unless the new `discussionrequireids` option is enabled (#2101)
+* CHANGED: Check `sizelimit` over full request, before attempting decoding
+* CHANGED: Database backend now logs errors when reading comments, like it already did for comment existence checks (#2100)
+* FIXED: Creating a document failed with a JavaScript error when the `discussion`, `password`, e-mail or QR code option is disabled, as the top navigation accessed the omitted elements
 * FIXED: Slovenian plural forms were stored in the wrong order, i.e. "1 uri" instead of "1 ura" for one hour (#1951)
 * FIXED: Lithuanian plural formula selected the "few" form for numbers ending in 0 above 10, i.e. "20 sekundės" instead of "20 sekundžių" (#1956)
+* FIXED: Comments must now be unique in cipher text within a document, not reuse existing cipher texts (#2100)
 
 ## 2.1.0 (2026-10-18)
 * ADDED: Added `shortenviachhoto` endpoint with an `chhoto` configuration section
 * ADDED: Translation for Chinese (Traditional)
 * ADDED: Subject line to the "Email" share button's mailto link (#928)
 * ADDED: Send the document with Ctrl+S, or Cmd+S on macOS (#739)
-* CHANGED: We removed jQuery in the Frontend and replaced it with vanilla JS.
-* CHANGED: Removed the unmaintained js-verify and replaced it with fast-check library.
-* CHANGED: Added a `jsconfig.json` in order to check the types of JavaScript.
-* CHANGED: Removed support for Bootstrap 3 as it requires jQuery.
+* CHANGED: We removed jQuery in the Frontend and replaced it with vanilla JS
+* CHANGED: Removed the unmaintained js-verify and replaced it with fast-check library
+* CHANGED: Added a `jsconfig.json` in order to check the types of JavaScript
+* CHANGED: Removed support for Bootstrap 3 as it requires jQuery
 * CHANGED: Encode documents with `TextEncoder` in `compress()`, instead of the `utf16To8()` percent-encoding round trip, which allocated ~20 times the payload
 * CHANGED: Upgrading libraries to: DOMpurify 3.4.17, ip-lib 1.22.1 & polyfill-php80 1.43.0
 * FIXED: Decode decompressed documents with `TextDecoder` instead of `utf8To16()`, which allocated ~3.4 GB for a 100 MB document and killed the tab (#2027)

+ 1 - 1
cfg/conf.sample.php

@@ -45,7 +45,7 @@ defaultformatter = "plaintext"
 ; syntaxhighlightingtheme = "sons-of-obsidian"
 
 ; size limit per document or comment in bytes, defaults to 10 Megabytes
-sizelimit = 10000000
+sizelimit = 10485760
 
 ; by default PrivateBin use "bootstrap5" template (tpl/bootstrap5.php).
 ; Optionally you can enable the template selection menu, which uses

+ 41 - 2
doc/Installation.md

@@ -141,10 +141,49 @@ you can also adjust an existing `robots.txt` file. More bot detection is
 implemented in JavaScript, but note none of these mechanisms is a 100% fail-safe
 way to prevent non-human visitors on your site.
 
+#### Server-side User-Agent blocking
+
+Client-side checks and `robots.txt` only help after a request reaches PrivateBin
+(or when a crawler chooses to honour `robots.txt`). You can additionally reject
+obvious crawler User-Agents in the web server, before PHP runs. That reduces
+noise in logs and lowers the chance that automated clients fetch paste URLs.
+
 A `.htaccess.disabled` file is provided in the root dir of PrivateBin. It blocks
 some known robots and link-scanning bots. If you use Apache, you can rename the
-file to `.htaccess` to enable this feature. If you use another webserver, you
-have to configure it manually to do the same.
+file to `.htaccess` to enable this feature. The shipped rules return HTTP 403
+when the User-Agent matches patterns such as `bot`, `spider`, `crawl`, embedded
+URLs, WhatsApp, Skype preview, or `facebookexternalhit`, while still allowing
+the Let's Encrypt validation User-Agent.
+
+If you use another web server, configure an equivalent deny list manually.
+For nginx, a `map` based on `$http_user_agent` is a common approach:
+
+```nginx
+map $http_user_agent $privatebin_bad_bot {
+    default                                                 0;
+    ~*Let's Encrypt validation server                        0;
+    ~*(?:bot|spider|crawl|https?://|WhatsApp|SkypeUriPreview|facebookexternalhit) 1;
+}
+
+server {
+    # ... existing PrivateBin server block ...
+
+    if ($privatebin_bad_bot) {
+        return 403;
+    }
+}
+```
+
+Tune the patterns for your environment. Broad matches such as `bot` can block
+legitimate tools or uptime monitors that put those substrings in their
+User-Agent. Prefer keeping health checks on simple HTTPS GETs that do not need
+to execute the client-side JavaScript bot detection.
+
+Operators of the official container images can apply the same idea in the
+image web-server config (for example nginx in
+[docker-nginx-fpm-alpine](https://github.com/PrivateBin/docker-nginx-fpm-alpine)
+or the Unit config in
+[docker-unit-alpine](https://github.com/PrivateBin/docker-unit-alpine)).
 
 ### On using Cloudflare
 

+ 4 - 14
lib/Controller.php

@@ -92,7 +92,7 @@ class Controller
      * Factory of instance models
      *
      * @access private
-     * @var    model
+     * @var    Model
      */
     private $_model;
 
@@ -100,7 +100,7 @@ class Controller
      * request
      *
      * @access private
-     * @var    request
+     * @var    Request
      */
     private $_request;
 
@@ -280,14 +280,15 @@ class Controller
         ServerSalt::setStore($this->_model->getStore());
         TrafficLimiter::setConfiguration($this->_conf);
         TrafficLimiter::setStore($this->_model->getStore());
+        $data = [];
         try {
             TrafficLimiter::canPass();
+            $data = $this->_request->getData($this->_conf->getKey('sizelimit'));
         } catch (TranslatedException $e) {
             $this->_json_error($e->getMessage());
             return;
         }
 
-        $data = $this->_request->getData();
         $isComment = array_key_exists('pasteid', $data) &&
             !empty($data['pasteid']) &&
             array_key_exists('parentid', $data) &&
@@ -296,17 +297,6 @@ class Controller
             $this->_json_error(I18n::_('Invalid data.'));
             return;
         }
-        $sizelimit = $this->_conf->getKey('sizelimit');
-        // Ensure content is not too big.
-        if (strlen($data['ct']) > $sizelimit) {
-            $this->_json_error(
-                I18n::_(
-                    'Document is limited to %s of encrypted data.',
-                    Filter::formatHumanReadableSize($sizelimit)
-                )
-            );
-            return;
-        }
 
         // The user posts a comment.
         if ($isComment) {

+ 9 - 4
lib/Data/Database.php

@@ -301,10 +301,15 @@ class Database extends AbstractData
      */
     public function readComments($pasteid)
     {
-        $rows = $this->_select(
-            'SELECT * FROM "' . $this->_sanitizeIdentifier('comment') .
-            '" WHERE "pasteid" = ?', [$pasteid]
-        );
+        try {
+            $rows = $this->_select(
+                'SELECT * FROM "' . $this->_sanitizeIdentifier('comment') .
+                '" WHERE "pasteid" = ?', [$pasteid]
+            );
+        } catch (PDOException $e) {
+            error_log('Error while reading comments from the database: ' . $e->getMessage());
+            return [];
+        }
 
         // create comment list
         $comments = [];

+ 21 - 18
lib/FormatV2.php

@@ -39,7 +39,7 @@ class FormatV2
             $required_keys[] = 'meta';
         }
 
-        // Make sure no additionnal keys were added.
+        // Make sure no additional keys were added.
         if (count(array_keys($message)) !== count($required_keys)) {
             return false;
         }
@@ -63,16 +63,29 @@ class FormatV2
             return false;
         }
 
+        // Make sure the adata parameters of pastes are a properly sized array
+        $adataLength = count($message['adata']);
+
         // Comments may authenticate their document and parent IDs, by
         // appending them to the cipher parameters. These must match the
         // unencrypted IDs, which are validated by the comment model.
-        if ($isComment && count($cipherParams) !== 8 && (
-            count($cipherParams) !== 10 ||
-            !is_string($cipherParams[8]) ||
-            !is_string($cipherParams[9]) ||
-            $cipherParams[8] !== $message['pasteid'] ||
-            $cipherParams[9] !== $message['parentid']
-        )) {
+        if ($isComment) {
+            if ($adataLength !== 8 && (
+                count($cipherParams) !== 10 ||
+                !is_string($cipherParams[8]) ||
+                !is_string($cipherParams[9]) ||
+                $cipherParams[8] !== $message['pasteid'] ||
+                $cipherParams[9] !== $message['parentid']
+            )) {
+                return false;
+            }
+        // For pastes: adata must have exactly 4 entries,
+        // and meta must contain only the 'expire' key.
+        } elseif ($adataLength !== 4 ||
+            !is_array($message['meta']) ||
+            count($message['meta']) !== 1 ||
+            !array_key_exists('expire', $message['meta'])
+        ) {
             return false;
         }
 
@@ -144,16 +157,6 @@ class FormatV2
             return false;
         }
 
-        // require only the key 'expire' in the metadata of pastes
-        if (!$isComment && (
-            !is_array($message['meta']) ||
-            count($message['meta']) === 0 ||
-            !array_key_exists('expire', $message['meta']) ||
-            count($message['meta']) > 1
-        )) {
-            return false;
-        }
-
         return true;
     }
 }

+ 20 - 0
lib/Model/Comment.php

@@ -56,6 +56,26 @@ class Comment extends AbstractModel
             throw new TranslatedException(self::COLLISION_ERROR, 69);
         }
 
+        // Comment IDs must be unique within the paste, regardless of the
+        // parent, and the parent must be either the paste or an existing comment.
+        $commentid = $this->getId();
+        $parentid = $this->getParentId();
+        $parentExists = $parentid === $pasteid;
+        if ($commentid === $pasteid) {
+            throw new TranslatedException(self::COLLISION_ERROR, 72);
+        }
+        foreach ($this->_store->readComments($pasteid) as $comment) {
+            if ($comment['id'] === $commentid) {
+                throw new TranslatedException(self::COLLISION_ERROR, 69);
+            }
+            if ($comment['id'] === $parentid) {
+                $parentExists = true;
+            }
+        }
+        if (!$parentExists) {
+            throw new TranslatedException(self::INVALID_DATA_ERROR, 71);
+        }
+
         $this->_data['meta']['created'] = time();
 
         // store comment

+ 20 - 1
lib/Request.php

@@ -13,6 +13,7 @@ namespace PrivateBin;
 
 use JsonException;
 use PrivateBin\Model\Paste;
+use PrivateBin\Exception\TranslatedException;
 
 /**
  * Request
@@ -42,6 +43,14 @@ class Request
      */
     const MIME_XHTML = 'application/xhtml+xml';
 
+    /**
+     * Input stream length for size checks
+     *
+     * @access private
+     * @var int
+     */
+    private $_bodyLength = 0;
+
     /**
      * Input stream to use for PUT parameter parsing
      *
@@ -112,6 +121,7 @@ class Request
                 $this->_operation = 'create';
                 try {
                     $data = file_get_contents(self::$_inputStream);
+                    $this->_bodyLength = strlen($data);
                     $this->_params = Json::decode($data);
                     // a valid JSON scalar (number, bool or string) decodes
                     // without error, but is not a usable set of parameters
@@ -196,10 +206,19 @@ class Request
      * Get data of paste or comment
      *
      * @access public
+     * @param  int $sizelimit
+     * @throws JsonException
+     * @throws TranslatedException
      * @return array
      */
-    public function getData()
+    public function getData(int $sizelimit)
     {
+        if ($this->_bodyLength > $sizelimit) {
+            throw new TranslatedException([
+                'Document is limited to %s of encrypted data.',
+                Filter::formatHumanReadableSize($sizelimit)
+            ]);
+        }
         $data = [
             'adata' => $this->getParam('adata'),
         ];

+ 9 - 1
tst/Bootstrap.php

@@ -75,7 +75,14 @@ class Helper
      *
      * @var string
      */
-    private static $commentid = '5a52eebf11c4c94b';
+    private static $commentid = '33a5671d11b29d01';
+
+    /**
+     * example comment cipher text, its hash is the comment ID
+     *
+     * @var string
+     */
+    private static $commentCt = 'XNE9E3AGszrjQfV5bxxSuaflS05MViI3rYtG1Nph0Qc=';
 
     /**
      * example comment meta data
@@ -162,6 +169,7 @@ class Helper
     {
         $example = self::$pasteV2;
         $example['adata'] = $example['adata'][0];
+        $example['ct'] = self::$commentCt;
         $example['pasteid'] = $example['parentid'] = self::getPasteId();
         unset($example['meta']['expire']);
         $example['meta'] = array_merge($example['meta'], self::$commentV2);

+ 30 - 3
tst/ControllerTest.php

@@ -273,6 +273,33 @@ class ControllerTest extends TestCase
         $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste exists after posting data');
     }
 
+    /**
+     * @runInSeparateProcess
+     */
+    public function testCreateOverSize()
+    {
+        $options                      = parse_ini_file(CONF, true);
+        $options['main']['sizelimit'] = 1000;
+        $options['traffic']['limit']  = 0;
+        Helper::createIniFile(CONF, $options);
+        $paste = Helper::getPastePost();
+        // add some extra authenticated data, could be a hypothetical future cryptographic hash, exceeding the sizelimit
+        $paste['adata'][] = str_repeat('0', $options['main']['sizelimit']);
+        $file  = Helper::createTempFile();
+        file_put_contents($file, json_encode($paste));
+        Request::setInputStream($file);
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
+        $_SERVER['REQUEST_METHOD']        = 'POST';
+        $_SERVER['REMOTE_ADDR']           = '::1';
+        ob_start();
+        new Controller;
+        $content = ob_get_contents();
+        ob_end_clean();
+        $response = json_decode($content, true);
+        $this->assertEquals(1, $response['status'], 'outputs error status');
+        $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste exists after posting data');
+    }
+
     /**
      * @runInSeparateProcess
      */
@@ -663,8 +690,8 @@ class ControllerTest extends TestCase
         $paste = Helper::getPaste();
         $this->_data->create(Helper::getPasteId(), $paste);
         $comment = Helper::getComment();
-        $this->_data->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId(), $comment);
-        $this->assertTrue($this->_data->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId()), 'comment exists before posting data');
+        $this->_data->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment);
+        $this->assertTrue($this->_data->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment exists before posting data');
         $comment = Helper::getCommentJson();
         $file = Helper::createTempFile();
         file_put_contents($file, $comment);
@@ -678,7 +705,7 @@ class ControllerTest extends TestCase
         ob_end_clean();
         $response = json_decode($content, true);
         $this->assertEquals(1, $response['status'], 'outputs error status');
-        $this->assertTrue($this->_data->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId()), 'paste exists after posting data');
+        $this->assertTrue($this->_data->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'paste exists after posting data');
     }
 
     /**

+ 8 - 0
tst/FormatV2Test.php

@@ -19,6 +19,14 @@ class FormatV2Test extends TestCase
         $paste['adata'][0][1] = '$';
         $this->assertFalse(FormatV2::isValid($paste), 'invalid base64 encoding of salt');
 
+        $paste = Helper::getPastePost();
+        $paste['adata'][] = 0;
+        $this->assertFalse(FormatV2::isValid($paste), 'invalid extra adata entry');
+
+        $paste = Helper::getPastePost();
+        $paste['adata'][] = 0;
+        $this->assertFalse(FormatV2::isValid($paste), 'invalid extra adata entry');
+
         $paste = Helper::getPastePost();
         $paste['ct'] = '$';
         $this->assertFalse(FormatV2::isValid($paste), 'invalid base64 encoding of ct');

+ 72 - 3
tst/ModelTest.php

@@ -91,7 +91,7 @@ class ModelTest extends TestCase
 
         $comments = $this->_model->getPaste(Helper::getPasteId())->get()['comments'];
         $this->assertTrue(count($comments) === 1, 'comment exists after storing it');
-        $commentData['id'] = Helper::getPasteId();
+        $commentData['id'] = Helper::getCommentId();
         $commentData['meta']['created'] = current($comments)['meta']['created'];
         $commentData['meta']['icon'] = current($comments)['meta']['icon'];
         $this->assertEquals($commentData, current($comments));
@@ -272,6 +272,75 @@ class ModelTest extends TestCase
         $comment->store();
     }
 
+    public function testCommentDuplicateOtherParent()
+    {
+        $pasteData = Helper::getPastePost();
+        $commentData = Helper::getCommentPost();
+        $this->_model->getPaste(Helper::getPasteId())->delete();
+
+        $paste = $this->_model->getPaste();
+        $paste->setData($pasteData);
+        $paste->store();
+
+        $comment = $paste->getComment(Helper::getPasteId());
+        $comment->setData($commentData);
+        $comment->store();
+        $firstCommentId = $comment->getId();
+
+        // reply to the first comment, using a different ciphertext
+        $replyData = $commentData;
+        $replyData['ct'] = base64_encode('reply ciphertext');
+        $replyData['parentid'] = $firstCommentId;
+        $reply = $paste->getComment($firstCommentId);
+        $reply->setData($replyData);
+        $reply->store();
+        $this->assertTrue($reply->exists(), 'reply to existing comment can be stored');
+
+        // replay of the first comment under the reply as parent
+        $commentData['parentid'] = $reply->getId();
+        $comment = $paste->getComment($reply->getId());
+        $comment->setData($commentData);
+        $this->expectException(Exception::class);
+        $this->expectExceptionCode(69);
+        $comment->store();
+    }
+
+    public function testCommentNonexistentParent()
+    {
+        $pasteData = Helper::getPastePost();
+        $commentData = Helper::getCommentPost();
+        $commentData['parentid'] = '1111111111111111';
+        $this->_model->getPaste(Helper::getPasteId())->delete();
+
+        $paste = $this->_model->getPaste();
+        $paste->setData($pasteData);
+        $paste->store();
+
+        $comment = $paste->getComment('1111111111111111');
+        $comment->setData($commentData);
+        $this->expectException(Exception::class);
+        $this->expectExceptionCode(71);
+        $comment->store();
+    }
+
+    public function testCommentEqualsPaste()
+    {
+        $pasteData = Helper::getPastePost();
+        $commentData = Helper::getCommentPost();
+        $commentData['ct'] = $pasteData['ct'];
+        $this->_model->getPaste(Helper::getPasteId())->delete();
+
+        $paste = $this->_model->getPaste();
+        $paste->setData($pasteData);
+        $paste->store();
+
+        $comment = $paste->getComment(Helper::getPasteId());
+        $comment->setData($commentData);
+        $this->expectException(Exception::class);
+        $this->expectExceptionCode(72);
+        $comment->store();
+    }
+
     public function testImplicitDefaults()
     {
         $pasteData = Helper::getPastePost();
@@ -460,14 +529,14 @@ class ModelTest extends TestCase
         $commentData = Helper::getCommentPost();
         unset($commentData['meta']['icon']);
         $paste = $model->getPaste(Helper::getPasteId());
-        $comment = $paste->getComment(Helper::getPasteId(), Helper::getPasteId());
+        $comment = $paste->getComment(Helper::getPasteId(), Helper::getCommentId());
         $this->assertFalse($comment->exists(), 'comment does not yet exist');
 
         $comment = $paste->getComment(Helper::getPasteId());
         $comment->setData($commentData);
         $comment->store();
 
-        $comment = $paste->getComment(Helper::getPasteId(), Helper::getPasteId());
+        $comment = $paste->getComment(Helper::getPasteId(), Helper::getCommentId());
         $this->assertTrue($comment->exists(), 'comment exists after storing it');
 
         $comment = current($this->_model->getPaste(Helper::getPasteId())->get()['comments']);