Skip to content

fix(security): keep caller-named fields off internal columns and operators #1056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abnegate wants to merge 11 commits into
base: fix/injection-hardening
Choose a base branch
from
+1,202 −319
Open

fix(security): keep caller-named fields off internal columns and operators #1056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e662482
fix(adapter): keep storage columns out of document writes that skip v…
abnegate Nov 5, 2026
6014651
fix(sql): run ddl one statement per prepare
abnegate Nov 5, 2026
129e2ac
fix(validator): stop allowlist regexes accepting a trailing newline
abnegate Nov 5, 2026
7981f21
fix(mongo): scope each read's maxTimeMS to its own event
abnegate Nov 5, 2026
8a3dfa6
fix(sql): let relationship ddl converge after a partial failure
abnegate Nov 5, 2026
d62d1af
fix(pool): drop a borrowed connection's stale per-event timeouts
abnegate Nov 5, 2026
51be506
fix(validator): refuse keys that reach a storage column once filtered
abnegate Nov 5, 2026
3240196
fix(memory): refuse increments of storage columns
abnegate Nov 5, 2026
f6e6615
docs: recommend disabling mysql multi-statements
abnegate Nov 5, 2026
c029eb4
style: drop narrating comments and abbreviated catch names
abnegate Nov 5, 2026
6bb4549
test(pool): expect a read adapter to clear every event's stale timeout
abnegate Nov 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ What an adapter supports is reported by `$database->getAdapter()->supports(Capab

`Utopia\Database\PDO` wraps PHP's PDO: it reconnects when a connection is lost outside a transaction and retries the call. Each SQL adapter also accepts a plain `PDO`. Pass the PDO attributes your application needs; the ones below are what the adapters expect.

The adapters send one statement per call, so disable MySQL multi-statements (on by default) with `\Pdo\Mysql::ATTR_MULTI_STATEMENTS => false` (PHP 8.4+) for MariaDB and MySQL; an injected `;` then cannot run a second statement.

#### MariaDB

```php
Expand All @@ -167,6 +169,7 @@ $pdo = new PDO("mysql:host={$host};port={$port};charset=utf8mb4", 'root', 'passw
\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
\PDO::ATTR_EMULATE_PREPARES => true,
\PDO::ATTR_STRINGIFY_FETCHES => true,
\Pdo\Mysql::ATTR_MULTI_STATEMENTS => false,
]);

$cache = new Cache(new Memory()); // or use any cache adapter you wish
Expand Down Expand Up @@ -194,6 +197,7 @@ $pdo = new PDO("mysql:host={$host};port={$port};charset=utf8mb4", 'root', 'passw
\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
\PDO::ATTR_EMULATE_PREPARES => true,
\PDO::ATTR_STRINGIFY_FETCHES => true,
\Pdo\Mysql::ATTR_MULTI_STATEMENTS => false,
]);

$cache = new Cache(new Memory()); // or use any cache adapter you wish
Expand Down
32 changes: 20 additions & 12 deletions src/Database/Adapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,24 +958,24 @@ protected function getInternalKeyForAttribute(string $attribute): string
}

/**
* Process-lifetime cache for {@see self::filter()}. Keys are referentially
* stable across the request lifetime and frequently re-queried per-row, so
* caching the regex result amortizes the preg_replace cost across all
* decode/encode/build passes. Bounded to avoid unbounded growth from
* unusual input.
*
* @var array<string, string>
*/
private static array $filteredKeyCache = [];

private const int FILTERED_KEY_CACHE_LIMIT = 4096;

/**
* Filter Keys
*
* @throws DatabaseException
*/
public function filter(string $value): string
{
return self::filterKey($value);
}

/**
* @throws DatabaseException
*/
public static function filterKey(string $value): string
{
if (isset(self::$filteredKeyCache[$value])) {
return self::$filteredKeyCache[$value];
Expand All @@ -995,10 +995,6 @@ public function filter(string $value): string
}

/**
* The column an attribute a caller names is written to. filter() keeps underscores, so a name such as `_tenant`
* or `_ten.ant` would reach a column the storage keeps for the document itself and move the document to another
* tenant, or rewrite its id or timestamps.
*
* @throws NotFoundException When the attribute names a reserved column
*/
protected function writableColumn(string $attribute): string
Expand All @@ -1012,6 +1008,18 @@ protected function writableColumn(string $attribute): string
return $column;
}

/**
* @return array<string, mixed>
*/
protected function writableAttributes(Document $document): array
{
return \array_filter(
$document->getAttributes(),
fn (int|string $key): bool => ! Storage::isReserved($this->filter((string) $key)),
\ARRAY_FILTER_USE_KEY
);
}

/**
* The row as every write hook decorates a row written for the document.
*
Expand Down
47 changes: 15 additions & 32 deletions src/Database/Adapter/MariaDB.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,9 @@ public function deleteCollection(string $collection): bool
$permissions = $schema->table($this->getTableRaw(Storage::permissionsTable($id)))->dropIfExists();

try {
return $this->executeStatement($main->query.'; '.$permissions->query, Event::CollectionDelete);
} catch (PDOException $e) {
$error = $this->processException($e);
return $this->executeStatements([$main->query, $permissions->query], Event::CollectionDelete);
} catch (PDOException $exception) {
$error = $this->processException($exception);
if ($error instanceof NotFoundException) {
$this->executeStatement($permissions->query, Event::CollectionDelete);
}
Expand Down Expand Up @@ -623,7 +623,7 @@ public function createDocument(Document $collection, Document $document): Docume

$spatialAttributes = $this->getSpatialAttributes($collection);
$collection = $collection->getId();
$attributes = $document->getAttributes();
$attributes = $this->writableAttributes($document);
$attributes[Storage::CREATED_AT] = $document->getCreatedAt();
$attributes[Storage::UPDATED_AT] = $document->getUpdatedAt();
$attributes[Storage::PERMISSIONS] = \json_encode($document->getPermissions());
Expand Down Expand Up @@ -671,27 +671,27 @@ public function createDocument(Document $collection, Document $document): Docume
$context = $this->writeContext();
try {
$this->runWriteHooks(fn ($hook) => $hook->afterDocumentCreate($name, [$document], $context));
} catch (PDOException $e) {
$isOrphanedPermission = $e->getCode() === '23000'
&& isset($e->errorInfo[1])
&& $e->errorInfo[1] === 1062
&& \str_contains($e->getMessage(), Storage::INDEX_1);
} catch (PDOException $exception) {
$isOrphanedPermission = $exception->getCode() === '23000'
&& isset($exception->errorInfo[1])
&& $exception->errorInfo[1] === 1062
&& \str_contains($exception->getMessage(), Storage::INDEX_1);

if (! $isOrphanedPermission) {
throw $e;
throw $exception;
}

// Clean up orphaned permissions from a previous failed delete, then retry
$cleanupBuilder = $this->newBuilder(Storage::permissionsTable($name));
$cleanupBuilder->filter([BaseQuery::equal(Storage::PERMISSIONS_DOCUMENT, [$document->getId()])]);
$cleanupResult = $cleanupBuilder->delete();
$cleanupStmt = $this->executeResult($cleanupResult, Event::PermissionsDelete);
$this->execute($cleanupStmt);
$cleanupStatement = $this->executeResult($cleanupResult, Event::PermissionsDelete);
$this->execute($cleanupStatement);

$this->runWriteHooks(fn ($hook) => $hook->afterDocumentCreate($name, [$document], $context));
}
} catch (PDOException $e) {
throw $this->processException($e);
} catch (PDOException $exception) {
throw $this->processException($exception);
}

return $document;
Expand All @@ -711,7 +711,7 @@ public function updateDocument(Document $collection, string $id, Document $docum

$spatialAttributes = $this->getSpatialAttributes($collection);
$collection = $collection->getId();
$attributes = $document->getAttributes();
$attributes = $this->writableAttributes($document);
$attributes[Storage::CREATED_AT] = $document->getCreatedAt();
$attributes[Storage::UPDATED_AT] = $document->getUpdatedAt();
$attributes[Storage::PERMISSIONS] = json_encode($document->getPermissions());
Expand Down Expand Up @@ -1065,23 +1065,6 @@ protected function decodePolygon(string $wkb): array
*/
private int $appliedRound = 0;

/**
* MySQL and MariaDB run every statement of a multi-statement string, but report only the first one's error from
* execute(); a later statement's error surfaces when its result is reached. Each result is reached here, so a
* statement that fails after another succeeded fails the call instead of being lost.
*/
#[\Override]
protected function executeStatement(string $sql, Event $event): bool
{
$statement = $this->prepareStatement($sql, $event);
$executed = $this->execute($statement);

while ($statement->nextRowset()) {
}

return $executed;
}

/**
* @param PDOStatement|DatabasePDOStatement|PDOStatementProxy $statement
*/
Expand Down
72 changes: 33 additions & 39 deletions src/Database/Adapter/Memory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1391,10 +1391,7 @@ public function updateDocument(Document $collection, string $id, Document $docum
}
[$oldKey, $existing] = $located;

// Resolve any Operator-typed attributes against the existing row before
// computing the new payload so unique-index checks see the post-update
// values, matching MariaDB's atomic UPDATE semantics.
$resolved = $this->applyOperators($document->getAttributes(), $existing);
$resolved = $this->applyOperators($this->writableAttributes($document), $existing);
foreach ($resolved as $attribute => $value) {
$document->setAttribute($attribute, $value);
}
Expand Down Expand Up @@ -1535,7 +1532,7 @@ public function updateDocuments(Document $collection, Document $updates, array $
throw new NotFoundException('Collection not found');
}

$attributes = $updates->getAttributes();
$attributes = $this->writableAttributes($updates);
$hasCreatedAt = ! empty($updates->getCreatedAt());
$hasUpdatedAt = ! empty($updates->getUpdatedAt());
$hasPermissions = $updates->offsetExists(Document::PERMISSIONS);
Expand All @@ -1547,14 +1544,14 @@ public function updateDocuments(Document $collection, Document $updates, array $
// before any write lands, so a uniqueness violation on document N
// does not leave documents 0..N-1 partially committed.
$prepared = [];
foreach ($documents as $doc) {
$uid = $doc->getId();
$docKey = $this->documentKey($uid);
if (! isset($this->data[$key]['documents'][$docKey])) {
foreach ($documents as $document) {
$uid = $document->getId();
$documentKey = $this->documentKey($uid);
if (! isset($this->data[$key]['documents'][$documentKey])) {
continue;
}

$existingRow = $this->data[$key]['documents'][$docKey];
$existingRow = $this->data[$key]['documents'][$documentKey];

// Resolve operators per-row — each document's existing values feed
// back into operator evaluation, so $attributes cannot be evaluated
Expand All @@ -1574,29 +1571,29 @@ public function updateDocuments(Document $collection, Document $updates, array $

$prepared[] = [
'uid' => $uid,
'docKey' => $docKey,
'attrs' => $resolved,
'documentKey' => $documentKey,
'attributes' => $resolved,
'newSignatures' => $newSignatures,
'oldSignatures' => $oldSignatures,
];
}

// Phase-1: hashed unique-index validation. For each pending row, probe
// the hash table — but recognise that any hashed entry currently
// pointing at the row's own docKey is about to be replaced by the new
// pointing at the row's own document key is about to be replaced by the new
// signature, so it is not a conflict. We also build a per-batch
// pending map so two siblings collapsing to the same value collide.
$pendingByIndex = [];
foreach ($prepared as $entry) {
$docKey = $entry['docKey'];
$documentKey = $entry['documentKey'];
foreach ($entry['newSignatures'] as $indexId => $hash) {
$existing = $this->uniqueIndexHashes[$key][$indexId][$hash] ?? null;
if ($existing !== null && $existing !== $docKey) {
if ($existing !== null && $existing !== $documentKey) {
// The entry's own pre-update hash binding does not count
// as a conflict — it'll be removed when its row is rewritten.
$existingIsSelf = false;
foreach ($prepared as $other) {
if ($other['docKey'] === $existing && ($other['oldSignatures'][$indexId] ?? null) === $hash) {
if ($other['documentKey'] === $existing && ($other['oldSignatures'][$indexId] ?? null) === $hash) {
$existingIsSelf = true;
break;
}
Expand All @@ -1605,22 +1602,22 @@ public function updateDocuments(Document $collection, Document $updates, array $
throw new UniqueException(UniqueException::MESSAGE);
}
}
if (isset($pendingByIndex[$indexId][$hash]) && $pendingByIndex[$indexId][$hash] !== $docKey) {
if (isset($pendingByIndex[$indexId][$hash]) && $pendingByIndex[$indexId][$hash] !== $documentKey) {
throw new UniqueException(UniqueException::MESSAGE);
}
$pendingByIndex[$indexId][$hash] = $docKey;
$pendingByIndex[$indexId][$hash] = $documentKey;
}
}

$tenant = $this->getTenant();
foreach ($prepared as $entry) {
$uid = $entry['uid'];
$docKey = $entry['docKey'];
$resolved = $entry['attrs'];
$documentKey = $entry['documentKey'];
$resolved = $entry['attributes'];

$previousRow = $this->data[$key]['documents'][$docKey];
$previousRow = $this->data[$key]['documents'][$documentKey];

$row = &$this->data[$key]['documents'][$docKey];
$row = &$this->data[$key]['documents'][$documentKey];
foreach ($resolved as $attribute => $value) {
$row[$this->filter($attribute)] = $value;
}
Expand All @@ -1636,8 +1633,8 @@ public function updateDocuments(Document $collection, Document $updates, array $
}
unset($row);

$this->journal(function () use ($key, $docKey, $previousRow): void {
$this->data[$key]['documents'][$docKey] = $previousRow;
$this->journal(function () use ($key, $documentKey, $previousRow): void {
$this->data[$key]['documents'][$documentKey] = $previousRow;
});

if ($hasPermissions) {
Expand All @@ -1656,7 +1653,7 @@ public function updateDocuments(Document $collection, Document $updates, array $
$indexId,
$entry['newSignatures'][$indexId] ?? null,
$entry['oldSignatures'][$indexId] ?? null,
$docKey,
$documentKey,
);
}
}
Expand Down Expand Up @@ -1899,15 +1896,15 @@ public function sum(Document $collection, string $attribute, array $queries = []
#[\Override]
public function increaseDocumentAttribute(Document $collection, string $id, string $attribute, int|float|string $value, string $updatedAt, int|float|string|null $min = null, int|float|string|null $max = null): bool
{
$column = $this->writableColumn($attribute);
$key = $this->key($collection->getId());
$docKey = $this->documentKey($id);
if (! isset($this->data[$key]['documents'][$docKey])) {
$documentKey = $this->documentKey($id);
if (! isset($this->data[$key]['documents'][$documentKey])) {
throw new NotFoundException('Document not found');
}

$column = $this->filter($attribute);
$previousValue = $this->data[$key]['documents'][$docKey][$column] ?? null;
$previousUpdatedAt = $this->data[$key]['documents'][$docKey][Storage::UPDATED_AT] ?? null;
$previousValue = $this->data[$key]['documents'][$documentKey][$column] ?? null;
$previousUpdatedAt = $this->data[$key]['documents'][$documentKey][Storage::UPDATED_AT] ?? null;
$current = $previousValue ?? 0;
$exact = (\is_int($current) || (\is_string($current) && BigInt::isIntegerString($current)))
&& (\is_int($value) || (\is_string($value) && BigInt::isIntegerString($value)));
Expand All @@ -1934,14 +1931,14 @@ public function increaseDocumentAttribute(Document $collection, string $id, stri
$result = $current + $value;
}

$this->data[$key]['documents'][$docKey][$column] = $result;
$this->data[$key]['documents'][$docKey][Storage::UPDATED_AT] = $updatedAt;
$this->data[$key]['documents'][$documentKey][$column] = $result;
$this->data[$key]['documents'][$documentKey][Storage::UPDATED_AT] = $updatedAt;

$this->journal(function () use ($key, $docKey, $column, $previousValue, $previousUpdatedAt): void {
if (! isset($this->data[$key]['documents'][$docKey])) {
$this->journal(function () use ($key, $documentKey, $column, $previousValue, $previousUpdatedAt): void {
if (! isset($this->data[$key]['documents'][$documentKey])) {
return;
}
$row = &$this->data[$key]['documents'][$docKey];
$row = &$this->data[$key]['documents'][$documentKey];
if ($previousValue === null) {
unset($row[$column]);
} else {
Expand Down Expand Up @@ -2026,10 +2023,7 @@ public function getAttributeWidth(Document $collection): int
protected function documentToRow(Document $document): array
{
$row = [];
foreach ($document->getAttributes() as $attribute => $value) {
// Store native PHP values directly — no JSON encoding for arrays.
// The Database casting layer accepts already-decoded arrays
// (decodeArrayValue / decodeObjectValue both pass through arrays).
foreach ($this->writableAttributes($document) as $attribute => $value) {
$row[$this->filter($attribute)] = $value;
}

Expand Down
Loading
Loading