[SECURITY] Harden PageRepository::findDirectSubpages (#926)

This fixes an SQL injection vulnerability.

Fixes #908

CHANGELOG.md

@@ -15,6 +15,11 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
+## 4.1.6
+
+### Fixed
+- Harden `PageRepository::findDirectSubpages` (#916)
+
 ## 4.1.5
 
 ### Added

Classes/Domain/Repository/PageRepository.php

@@ -5,6 +5,7 @@
 namespace OliverKlee\Oelib\Domain\Repository;
 
 use Doctrine\DBAL\Driver\ResultStatement;
+use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\QueryBuilder;
 use TYPO3\CMS\Core\SingletonInterface;
@@ -32,8 +33,7 @@ public function findWithinParentPages(array $pageUids, int $recursion = 0): arra
         if ($recursion < 0) {
             throw new \InvalidArgumentException('$recursion must be >= 0, but actually is: ' . $recursion, 1608389744);
         }
-        $result = $pageUids;
-        \sort($result, SORT_NUMERIC);
+        $result = $this->cleanUids($pageUids);
         if ($result === [] || $recursion === 0) {
             return $result;
         }
@@ -48,14 +48,38 @@ public function findWithinParentPages(array $pageUids, int $recursion = 0): arra
     }
 
     /**
-     * @param array<array-key, positive-int> $pageUids
+     * Filters and int-casts the given UIDs and returns only positive integers, discarding the rest.
+     *
+     * @param array <array-key, int|string> $uids
+     *
+     * @return array<int, positive-int> sorted, filtered UIDs
+     */
+    private function cleanUids(array $uids): array
+    {
+        $cleanUids = [];
+        foreach ($uids as $uid) {
+            $intUid = (int)$uid;
+            if ($intUid > 0) {
+                $cleanUids[] = $intUid;
+            }
+        }
+        \sort($cleanUids, SORT_NUMERIC);
+
+        return $cleanUids;
+    }
+
+    /**
+     * @param array<int, positive-int> $pageUids
      *
      * @return array<int, positive-int>
      */
     private function findDirectSubpages(array $pageUids): array
     {
-        $query = $this->getQueryBuilderForTable('pages')->select('uid')->from('pages');
-        $query->andWhere($query->expr()->in('pid', $pageUids));
+        $queryBuilder = $this->getQueryBuilderForTable('pages');
+        $query = $queryBuilder->select('uid')->from('pages');
+        $query->andWhere(
+            $query->expr()->in('pid', $queryBuilder->createNamedParameter($pageUids, Connection::PARAM_INT_ARRAY))
+        );
 
         $subpageUids = [];
         $queryResult = $query->execute();

Tests/Functional/Domain/Repository/PageRepositoryTest.php

@@ -190,4 +190,43 @@ public function findWithinParentPagesWithSubpagesSortsResult(): void
 
         self::assertSame([4, 5, 11, 12, 13], $result);
     }
+
+    /**
+     * @return array<string, array{0: string|int}>
+     */
+    public function invalidUidDataProvider(): array
+    {
+        return [
+            'empty string' => [''],
+            'non-empty string' => ['Bratwurst'],
+            'zero' => [0],
+            'negative int' => [-1],
+        ];
+    }
+
+    /**
+     * @test
+     */
+    public function findWithinParentPagesSilentlyCastsIntLikeUidStringsToInt(): void
+    {
+        // @phpstan-ignore-next-line We are explicitly testing with the contract violation here.
+        $result = $this->subject->findWithinParentPages(['11'], 2);
+
+        self::assertSame([11, 12, 13], $result);
+    }
+
+    /**
+     * @test
+     *
+     * @param string|int $invalidUid
+     *
+     * @dataProvider invalidUidDataProvider
+     */
+    public function findWithinParentPagesWithSubpagesSilentlyDropsInvalidUids($invalidUid): void
+    {
+        // @phpstan-ignore-next-line We are explicitly testing with the contract violation here.
+        $result = $this->subject->findWithinParentPages([1, $invalidUid]);
+
+        self::assertSame([1], $result);
+    }
 }