Merge remote-tracking branch 'advisory-fix-1'

CHANGELOG.md

@@ -1,8 +1,9 @@
 # PrivateBin version history
 
 ## 2.1.1 (not yet released)
-* 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)
+* CHANGED: Check `sizelimit` over full request, before attempting decoding
+* 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)
 
@@ -11,10 +12,10 @@
 * 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)

cfg/conf.sample.php

@@ -40,7 +40,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

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) {

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,6 +63,11 @@ class FormatV2
             return false;
         }
 
+        // Make sure the adata parameters of pastes are a properly sized array,
+        if (!$isComment && count($message['adata']) !== 4) {
+            return false;
+        }
+
         // Make sure the ciphertext and the cipher parameters used in the
         // string operations below are actually strings, so that malformed
         // input yields "Invalid data." instead of a fatal type error.

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'),
         ];

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
      */

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');