- Issue created by @tedbow
Part of
🌱
[Meta] Address low risk issues found in security Audit
Active
Cure53 noted the presence of a Time-Of-Check Time-Of-Use (TOCTOU) race condition within the logic that acquires the lock in the temporary storage before creating a stage. The logic asserts the availability of a key within the key store via the isAvailable method (TOC) and proceeds to assign a value a few lines later (TOU). Those two operations are not executed atomically and lack any other form of synchronization, which therefore allows for a race condition whereby two concurrent execution runs can pass the availability check before one of them holds the lock before the other. In rare edge cases, this renders the locking mechanism ineffective and could lead to multiple stages being committed simultaneously.
Affected file:
automatic_updates/package_manager/src/StageBase.php
Affected code:
public function create(?int $timeout = 300): string {
[...]
if (!$this->isAvailable()) {
throw new StageException($this, 'Cannot create [...] already
exists.'); }
[...] $this->tempStore->set(static::TEMPSTORE_LOCK_KEY, [
$id,
static::class,
$this->getType(),
]); [...]
final public function isAvailable(): bool {
return empty($this->tempStore->getMetadata(static::TEMPSTORE_LOCK_KEY));
}
To mitigate this issue, Cure53 recommends employing an adequate locking technique as offered by the acquire method of the Drupal\Core\Lock namespace’s LockBackendInterface interface. If the implementation guarantees that this lock can only be held by a single instance at the same time, the aforementioned flaw will be neutralized.
Active
3.0
Code