Merge pull request #1438 from PrivateBin/array-key-type-error

Numeric array key type errors

CHANGELOG.md

@@ -7,6 +7,7 @@
 * CHANGED: Simpler PostgreSQL table lookup query (#1361)
 * CHANGED: SRI hashes are now configurable, no longer hardcoded in templates (#1365)
 * CHANGED: Upgrading libraries to: DOMpurify 3.1.7
+* FIXED: Numeric array keys being cast to integer causing failures under strict type checking (#1435)
 
 ## 1.7.4 (2024-07-09)
 * CHANGED: Saving markdown pastes uses `.md` extension instead of `.txt` (#1293)

lib/Data/Database.php

@@ -506,8 +506,8 @@ class Database extends AbstractData
     private function _exec($sql, array $params)
     {
         $statement = $this->_db->prepare($sql);
-        foreach ($params as $key => &$parameter) {
-            $position = $key + 1;
+        $position  = 1;
+        foreach ($params as &$parameter) {
             if (is_int($parameter)) {
                 $statement->bindParam($position, $parameter, PDO::PARAM_INT);
             } elseif (is_string($parameter) && strlen($parameter) >= 4000) {
@@ -515,6 +515,7 @@ class Database extends AbstractData
             } else {
                 $statement->bindParam($position, $parameter);
             }
+            ++$position;
         }
         $result = $statement->execute();
         $statement->closeCursor();

lib/Request.php

@@ -83,6 +83,7 @@ class Request
     {
         foreach ($_GET as $key => $value) {
             // only return if value is empty and key is 16 hex chars
+            $key = (string) $key;
             if (($value === '') && strlen($key) === 16 && ctype_xdigit($key)) {
                 return $key;
             }

tst/Bootstrap.php

@@ -116,7 +116,7 @@ class Helper
      *
      * @return string
      */
-    public static function getPasteId()
+    public static function getPasteId(): string
     {
         return self::$pasteid;
     }
@@ -128,7 +128,7 @@ class Helper
      * @param  array $meta
      * @return array
      */
-    public static function getPaste($version = 2, array $meta = array())
+    public static function getPaste($version = 2, array $meta = array()): array
     {
         $example = self::getPasteWithAttachment($version, $meta);
         // v1 has the attachment stored in a separate property
@@ -145,7 +145,7 @@ class Helper
      * @param  array $meta
      * @return array
      */
-    public static function getPasteWithAttachment($version = 2, array $meta = array())
+    public static function getPasteWithAttachment($version = 2, array $meta = array()): array
     {
         $example                 = $version === 1 ? self::$pasteV1 : self::$pasteV2;
         $example['meta']['salt'] = ServerSalt::generate();
@@ -160,7 +160,7 @@ class Helper
      * @param  array $meta
      * @return array
      */
-    public static function getPastePost($version = 2, array $meta = array())
+    public static function getPastePost($version = 2, array $meta = array()): array
     {
         $example         = self::getPaste($version, $meta);
         if ($version == 2) {
@@ -176,9 +176,9 @@ class Helper
      *
      * @param  int $version
      * @param  array $meta
-     * @return array
+     * @return string
      */
-    public static function getPasteJson($version = 2, array $meta = array())
+    public static function getPasteJson($version = 2, array $meta = array()): string
     {
         return json_encode(self::getPastePost($version, $meta));
     }
@@ -188,7 +188,7 @@ class Helper
      *
      * @return string
      */
-    public static function getCommentId()
+    public static function getCommentId(): string
     {
         return self::$commentid;
     }
@@ -200,7 +200,7 @@ class Helper
      * @param  array $meta
      * @return array
      */
-    public static function getComment($version = 2, array $meta = array())
+    public static function getComment($version = 2, array $meta = array()): array
     {
         $example         = $version === 1 ? self::$commentV1 : self::$pasteV2;
         if ($version === 2) {
@@ -220,7 +220,7 @@ class Helper
      * @param  int $version
      * @return array
      */
-    public static function getCommentPost()
+    public static function getCommentPost(): array
     {
         $example = self::getComment();
         unset($example['meta']);
@@ -231,20 +231,32 @@ class Helper
      * get example comment, as received via POST by user
      *
      * @param  int $version
-     * @return array
+     * @return string
      */
-    public static function getCommentJson()
+    public static function getCommentJson(): string
     {
         return json_encode(self::getCommentPost());
     }
 
+    /**
+     * Returns 16 random hexadecimal characters.
+     *
+     * @return string
+     */
+    public static function getRandomId(): string
+    {
+        // 8 binary bytes are 16 characters long in hex
+        return bin2hex(random_bytes(8));
+    }
+
     /**
      * delete directory and all its contents recursively
      *
      * @param string $path
      * @throws Exception
+     * @return void
      */
-    public static function rmDir($path)
+    public static function rmDir($path): void
     {
         if (is_dir($path)) {
             $path .= DIRECTORY_SEPARATOR;
@@ -272,7 +284,7 @@ class Helper
      *
      * @return void
      */
-    public static function confBackup()
+    public static function confBackup(): void
     {
         if (!is_file(CONF . '.bak') && is_file(CONF)) {
             rename(CONF, CONF . '.bak');
@@ -287,7 +299,7 @@ class Helper
      *
      * @return void
      */
-    public static function confRestore()
+    public static function confRestore(): void
     {
         if (is_file(CONF . '.bak')) {
             rename(CONF . '.bak', CONF);
@@ -303,7 +315,7 @@ class Helper
      * @param string $pathToFile
      * @param array $values
      */
-    public static function createIniFile($pathToFile, array $values)
+    public static function createIniFile($pathToFile, array $values): void
     {
         if (count($values)) {
             @unlink($pathToFile);
@@ -346,7 +358,7 @@ class Helper
      * @param bool $return
      * @return void|string
      */
-    public static function varExportMin($var, $return = false)
+    public static function varExportMin($var, $return = false): string
     {
         if (is_array($var)) {
             $toImplode = array();
@@ -369,7 +381,7 @@ class Helper
      *
      * @return void
      */
-    public static function updateSubresourceIntegrity()
+    public static function updateSubresourceIntegrity(): void
     {
         foreach (new GlobIterator(PATH . 'js' . DIRECTORY_SEPARATOR . '*.js') as $file) {
             if ($file->getBasename() == 'common.js') {

tst/Data/FilesystemTest.php

@@ -141,9 +141,7 @@ class FilesystemTest extends TestCase
         $commentid = Helper::getCommentId();
         $ids       = array();
         for ($i = 0, $max = 10; $i < $max; ++$i) {
-            // PHPs mt_rand only supports 32 bit or up 0x7fffffff on 64 bit systems to be precise :-/
-            $dataid = str_pad(dechex(mt_rand(0, mt_getrandmax())), 8, '0', STR_PAD_LEFT) .
-                str_pad(dechex(mt_rand(0, mt_getrandmax())), 8, '0', STR_PAD_LEFT);
+            $dataid     = Helper::getRandomId();
             $storagedir = $this->_path . DIRECTORY_SEPARATOR . substr($dataid, 0, 2) .
                 DIRECTORY_SEPARATOR . substr($dataid, 2, 2) . DIRECTORY_SEPARATOR;
             $ids[$dataid] = $storagedir;

tst/RequestTest.php

@@ -12,18 +12,6 @@ class RequestTest extends TestCase
         $_POST   = array();
     }
 
-    /**
-     * Returns 16 random hexadecimal characters.
-     *
-     * @access public
-     * @return string
-     */
-    public function getRandomId()
-    {
-        // 8 binary bytes are 16 characters long in hex
-        return bin2hex(random_bytes(8));
-    }
-
     /**
      * Returns random query safe characters.
      *
@@ -54,7 +42,25 @@ class RequestTest extends TestCase
     public function testRead()
     {
         $this->reset();
-        $id                        = $this->getRandomId();
+        $id                        = Helper::getRandomId();
+        $_SERVER['REQUEST_METHOD'] = 'GET';
+        $_SERVER['QUERY_STRING']   = $id;
+        $_GET[$id]                 = '';
+        $request                   = new Request;
+        $this->assertFalse($request->isJsonApiCall(), 'is HTML call');
+        $this->assertEquals($id, $request->getParam('pasteid'));
+        $this->assertEquals('read', $request->getOperation());
+    }
+
+    /**
+     * paste IDs are 8 bytes hex encoded strings, if unlucky, this turns into
+     * a numeric string that PHP will cast to an int, for example in array keys
+     * @see https://www.php.net/manual/en/language.types.array.php
+     */
+    public function testReadNumeric()
+    {
+        $this->reset();
+        $id                        = '1234567812345678';
         $_SERVER['REQUEST_METHOD'] = 'GET';
         $_SERVER['QUERY_STRING']   = $id;
         $_GET[$id]                 = '';
@@ -67,7 +73,7 @@ class RequestTest extends TestCase
     public function testDelete()
     {
         $this->reset();
-        $id                        = $this->getRandomId();
+        $id                        = Helper::getRandomId();
         $_SERVER['REQUEST_METHOD'] = 'GET';
         $_GET['pasteid']           = $id;
         $_GET['deletetoken']       = 'bar';
@@ -110,7 +116,7 @@ class RequestTest extends TestCase
     public function testApiRead()
     {
         $this->reset();
-        $id                        = $this->getRandomId();
+        $id                        = Helper::getRandomId();
         $_SERVER['REQUEST_METHOD'] = 'GET';
         $_SERVER['HTTP_ACCEPT']    = 'application/json, text/javascript, */*; q=0.01';
         $_SERVER['QUERY_STRING']   = $id;
@@ -124,7 +130,7 @@ class RequestTest extends TestCase
     public function testApiDelete()
     {
         $this->reset();
-        $id                               = $this->getRandomId();
+        $id                               = Helper::getRandomId();
         $_SERVER['REQUEST_METHOD']        = 'POST';
         $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
         $_SERVER['QUERY_STRING']          = $id;
@@ -155,7 +161,7 @@ class RequestTest extends TestCase
     public function testReadWithNegotiation()
     {
         $this->reset();
-        $id                        = $this->getRandomId();
+        $id                        = Helper::getRandomId();
         $_SERVER['REQUEST_METHOD'] = 'GET';
         $_SERVER['HTTP_ACCEPT']    = 'text/html,text/html; charset=UTF-8,application/xhtml+xml, application/xml;q=0.9,*/*;q=0.8, text/csv,application/json';
         $_SERVER['QUERY_STRING']   = $id;
@@ -169,7 +175,7 @@ class RequestTest extends TestCase
     public function testReadWithXhtmlNegotiation()
     {
         $this->reset();
-        $id                        = $this->getRandomId();
+        $id                        = Helper::getRandomId();
         $_SERVER['REQUEST_METHOD'] = 'GET';
         $_SERVER['HTTP_ACCEPT']    = 'application/xhtml+xml,text/html,text/html; charset=UTF-8, application/xml;q=0.9,*/*;q=0.8, text/csv,application/json';
         $_SERVER['QUERY_STRING']   = $id;
@@ -183,7 +189,7 @@ class RequestTest extends TestCase
     public function testApiReadWithNegotiation()
     {
         $this->reset();
-        $id                        = $this->getRandomId();
+        $id                        = Helper::getRandomId();
         $_SERVER['REQUEST_METHOD'] = 'GET';
         $_SERVER['HTTP_ACCEPT']    = 'text/plain,text/csv, application/xml;q=0.9, application/json, text/html,text/html; charset=UTF-8,application/xhtml+xml, */*;q=0.8';
         $_SERVER['QUERY_STRING']   = $id;
@@ -197,7 +203,7 @@ class RequestTest extends TestCase
     public function testReadWithFailedNegotiation()
     {
         $this->reset();
-        $id                        = $this->getRandomId();
+        $id                        = Helper::getRandomId();
         $_SERVER['REQUEST_METHOD'] = 'GET';
         $_SERVER['HTTP_ACCEPT']    = 'text/plain,text/csv, application/xml;q=0.9, */*;q=0.8';
         $_SERVER['QUERY_STRING']   = $id;
@@ -211,7 +217,7 @@ class RequestTest extends TestCase
     public function testPasteIdExtraction()
     {
         $this->reset();
-        $id              = $this->getRandomId();
+        $id              = Helper::getRandomId();
         $queryParams     = array($id);
         $queryParamCount = random_int(1, 5);
         for ($i = 0; $i < $queryParamCount; ++$i) {