copenhagentruckwash/api
1
0
Fork 0
Code Issues Pull Requests 1 Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #237 from copenhagentruckwash/fix-concurrent-access-vulnerability-in-start-command

Browse Source 
Add per-lane START lock to prevent TOCTOU relay replay on wash start
This commit is contained in:
Jeppe B
2026-06-01 23:28:45 +02:00
committed by GitHub
parent bfec31f94b bffed6f5f3
commit 07a3ef6418
2 changed files with 111 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/nginx/app/modules/selfserve/traits/selfserve_lane_command_t.php
+83 -30
View File
@@ -38,6 +38,49 @@ use objects\department_variables_o;
trait selfserve_lane_command_t
{
/**
* Acquire an atomic per-lane START lock before performing physical side effects.
*/
protected function acquireLaneStartCommandLock(): string
{
if (!defined('redis') || !method_exists(redis, 'set_if_absent_with_expiration')) {
throw new \RuntimeException('Cannot start lane: START lock is unavailable.');
}
$token = bin2hex(random_bytes(16));
$lock_key = $this->getLaneStartCommandLockKey();
if (!redis->set_if_absent_with_expiration($lock_key, $token, 30)) {
throw new \RuntimeException("Cannot start lane: Lane is not available.");
}
return $token;
}
protected function releaseLaneStartCommandLock(string $token): void
{
if (!defined('redis')) {
return;
}
$lock_key = $this->getLaneStartCommandLockKey();
try {
if (method_exists(redis, 'get') && redis->get($lock_key) !== $token) {
return;
}
if (method_exists(redis, 'delete')) {
redis->delete($lock_key);
}
} catch (\Throwable) {
// The lock has a short TTL, so release failures must not mask START results.
}
}
protected function getLaneStartCommandLockKey(): string
{
return 'selfserve_lane_start_command_lock_' . (int)$this->id;
}
/**
* Determine if the lane and its department have self-serve enabled.
* This method is intentionally protected to allow tests to override
@@ -509,38 +552,48 @@ trait selfserve_lane_command_t
// Validate customer number
if (!is_numeric($customer_number) || (int)$customer_number <= 0) throw new \InvalidArgumentException("Invalid customer number: " . $customer_number);
if (!(new users_o())->getUserByCustomerNumber((int)$customer_number)->exists()) throw new \InvalidArgumentException("Customer number does not exist: " . $customer_number);
$previous_customer_number = $this->getCustomerNumber();
$previous_license_plate = $this->getLicensePlate();
// Set the customer number and license plate
$this->setCustomerNumber($customer_number);
$this->setLicensePlate($license_plate);
$start_lock_token = $this->acquireLaneStartCommandLock();
try {
// Open the entrance port before marking the lane occupied. Gateway timeouts are
// ambiguous because the relay may already have received the pulse.
$this->openEntrancePortForWashStart();
} catch (\Throwable $e) {
$this->setCustomerNumber($previous_customer_number);
$this->setLicensePlate($previous_license_plate);
$this->setLaneState(selfserve_lane_state::IDLE);
throw $e;
// Re-check availability after taking the START lock so concurrent requests cannot
// both pass the preflight check and trigger the physical entrance relay.
if (!$this->getLaneStatus()->equals(selfserve_lane_status::AVAILABLE)) throw new \RuntimeException("Cannot start lane: Lane is not available.");
$previous_customer_number = $this->getCustomerNumber();
$previous_license_plate = $this->getLicensePlate();
$previous_status = $this->getLaneStatus();
// Set the customer number and license plate
$this->setCustomerNumber($customer_number);
$this->setLicensePlate($license_plate);
// Mark the lane occupied before any physical entrance relay side effects.
$this->setLaneStatus(selfserve_lane_status::OCCUPIED);
try {
// Gateway timeouts are ambiguous because the relay may already have received
// the pulse, so openEntrancePortForWashStart() reports them and continues.
$this->openEntrancePortForWashStart();
} catch (\Throwable $e) {
$this->setCustomerNumber($previous_customer_number);
$this->setLicensePlate($previous_license_plate);
$this->setLaneStatus($previous_status);
$this->setLaneState(selfserve_lane_state::IDLE);
throw $e;
}
// Set the lane state to IN_WASH
$this->setLaneState(selfserve_lane_state::IN_WASH);
// Start the wash timer
$this->setWashStartTime(time());
$this->runPublishedStudioActions(
selfserve_studio_actions::EVENT_WASH_START_COMMAND,
$this->resolveSelfServeActionWashModeForStart(),
[
'customer_number' => (int)$customer_number,
'reg' => $license_plate,
]
);
$this->runRelaySideEffectsForWashStart($arguments);
// Log the lane start event
$this->logLaneAction(selfserve_lane_log_action::START_WASH);
} finally {
$this->releaseLaneStartCommandLock($start_lock_token);
}
// Set the lane status to OCCUPIED when started
$this->setLaneStatus(selfserve_lane_status::OCCUPIED);
// Set the lane state to IN_WASH
$this->setLaneState(selfserve_lane_state::IN_WASH);
// Start the wash timer
$this->setWashStartTime(time());
$this->runPublishedStudioActions(
selfserve_studio_actions::EVENT_WASH_START_COMMAND,
$this->resolveSelfServeActionWashModeForStart(),
[
'customer_number' => (int)$customer_number,
'reg' => $license_plate,
]
);
$this->runRelaySideEffectsForWashStart($arguments);
// Log the lane start event
$this->logLaneAction(selfserve_lane_log_action::START_WASH);
break;
case selfserve_lane_command::STOP:
// Require lane to be occupied before stopping
services/nginx/app/tests/Unit/Selfserve/SelfserveLaneStartRaceLockWiringTest.php
+28
View File
@@ -0,0 +1,28 @@
<?php
it('locks and marks a self-serve start occupied before opening the entrance relay', function (): void {
$commandTrait = file_get_contents(app_path('modules/selfserve/traits/selfserve_lane_command_t.php'));
expect($commandTrait)->not->toBeFalse();
expect($commandTrait)->toContain('set_if_absent_with_expiration');
$startCaseOffset = strpos($commandTrait, 'case selfserve_lane_command::START:');
expect($startCaseOffset)->not->toBeFalse();
$startCase = substr($commandTrait, (int)$startCaseOffset, 3500);
$lockOffset = strpos($startCase, '$start_lock_token = $this->acquireLaneStartCommandLock();');
$occupiedOffset = strpos($startCase, '$this->setLaneStatus(selfserve_lane_status::OCCUPIED);');
$openOffset = strpos($startCase, '$this->openEntrancePortForWashStart();');
$finallyOffset = strpos($startCase, '} finally {');
$releaseOffset = strpos($startCase, '$this->releaseLaneStartCommandLock($start_lock_token);');
expect($lockOffset)->not->toBeFalse()
->and($occupiedOffset)->not->toBeFalse()
->and($openOffset)->not->toBeFalse()
->and($finallyOffset)->not->toBeFalse()
->and($releaseOffset)->not->toBeFalse()
->and($lockOffset)->toBeLessThan($occupiedOffset)
->and($occupiedOffset)->toBeLessThan($openOffset)
->and($openOffset)->toBeLessThan($finallyOffset)
->and($finallyOffset)->toBeLessThan($releaseOffset);
});