Merge pull request #246 from copenhagentruckwash/fix-unbounded-relay-timer-vulnerability

Cap self-serve gate relay timers
This commit is contained in:
Jeppe B
2026-06-01 23:45:39 +02:00
committed by GitHub
6 changed files with 51 additions and 2 deletions
+2 -1
View File
@@ -18,6 +18,7 @@ const DEFAULT_UPDATE_VERIFY_INTERVAL_MS = 500;
const DEFAULT_UPDATE_RESTART_GRACE_MS = 150; const DEFAULT_UPDATE_RESTART_GRACE_MS = 150;
const DEFAULT_BROKER_RECONNECT_DELAY_MS = 1500; const DEFAULT_BROKER_RECONNECT_DELAY_MS = 1500;
const DEFAULT_SHELLY_LOCAL_HTTP_TIMEOUT_MS = 1200; const DEFAULT_SHELLY_LOCAL_HTTP_TIMEOUT_MS = 1200;
const MAX_RELAY_TOGGLE_AFTER_SECONDS = 5;
const UPDATE_VERIFY_COMMAND = "post-update-verify"; const UPDATE_VERIFY_COMMAND = "post-update-verify";
const execFile = promisify(execFileCallback); const execFile = promisify(execFileCallback);
@@ -482,7 +483,7 @@ function resolveRelayToggleAfterSeconds(payload = {}) {
return null; return null;
} }
return Math.floor(configured); return Math.min(Math.floor(configured), MAX_RELAY_TOGGLE_AFTER_SECONDS);
} }
async function fetchJson(url, fetchImpl = fetch, options = {}) { async function fetchJson(url, fetchImpl = fetch, options = {}) {
+25
View File
@@ -271,6 +271,31 @@ test("relay switch commands pass timer values to local Shelly APIs", async () =>
"http://10.1.0.31/rpc/Switch.Set?id=0&on=true&toggle_after=3", "http://10.1.0.31/rpc/Switch.Set?id=0&on=true&toggle_after=3",
"http://10.1.0.31/relay/0?turn=on&timer=3", "http://10.1.0.31/relay/0?turn=on&timer=3",
]); ]);
const cappedUrls = [];
const cappedFetch = async (url) => {
cappedUrls.push(String(url));
return {
ok: true,
async json() {
return { output: true };
},
};
};
await setRelayState({
localIp: "10.1.0.31",
channel: 0,
on: true,
toggle_after: 999999999,
device_generation: 3,
}, cappedFetch);
assert.equal(
cappedUrls[0],
"http://10.1.0.31/rpc/Switch.Set?id=0&on=true&toggle_after=5"
);
}); });
test("runUpdate stages a pending verification restart after installing new artifacts", async () => { test("runUpdate stages a pending verification restart after installing new artifacts", async () => {
@@ -17,6 +17,7 @@ trait selfserve_lane_port_controller_t
{ {
private const DEMO_RELAY_ID_PREFIX = 'demo-'; private const DEMO_RELAY_ID_PREFIX = 'demo-';
private const DEFAULT_PORT_OPEN_TOGGLE_AFTER_SECONDS = 1; private const DEFAULT_PORT_OPEN_TOGGLE_AFTER_SECONDS = 1;
private const MAX_PORT_OPEN_TOGGLE_AFTER_SECONDS = 5;
/** /**
* Open the lane port * Open the lane port
@@ -105,7 +106,7 @@ trait selfserve_lane_port_controller_t
private function normalizePortOpenToggleAfter(?int $toggle_after_seconds): int private function normalizePortOpenToggleAfter(?int $toggle_after_seconds): int
{ {
if ($toggle_after_seconds !== null && $toggle_after_seconds > 0) { if ($toggle_after_seconds !== null && $toggle_after_seconds > 0) {
return $toggle_after_seconds; return min($toggle_after_seconds, self::MAX_PORT_OPEN_TOGGLE_AFTER_SECONDS);
} }
return self::DEFAULT_PORT_OPEN_TOGGLE_AFTER_SECONDS; return self::DEFAULT_PORT_OPEN_TOGGLE_AFTER_SECONDS;
@@ -30,6 +30,7 @@ class moduleSelfServeRoute
{ {
use route_t; use route_t;
private const MAX_GATE_OPEN_TOGGLE_AFTER_SECONDS = 5;
private const CUSTOMER_SELFSERVE_PERMISSION = 'list_own_department_selfserve_vehicle_conditions'; private const CUSTOMER_SELFSERVE_PERMISSION = 'list_own_department_selfserve_vehicle_conditions';
public function run(): void public function run(): void
@@ -1596,6 +1597,7 @@ class moduleSelfServeRoute
} }
self::requireMinValue($toggle_after, 1); self::requireMinValue($toggle_after, 1);
self::requireMaxValue($toggle_after, self::MAX_GATE_OPEN_TOGGLE_AFTER_SECONDS);
return $toggle_after; return $toggle_after;
} }
@@ -152,6 +152,24 @@ it('passes explicit timer values when opening lane gates', function (): void {
]); ]);
}); });
it('caps excessive timer values when opening lane gates', function (): void {
$lane = new SelfserveLanePortControllerHarness(
relayInId: 'relay-in-123',
relayOutId: 'relay-out-123'
);
$result = $lane->open(selfserve_lane_port::ENTRANCE, 999999999);
expect($result)->toBeTrue();
expect($lane->switchFake->switchCalls)->toBe([
[
'id' => 'relay-in-123',
'on' => true,
'toggle_after' => 5,
],
]);
});
it('keeps demo relay gate-open queued but skips Shelly switch calls', function (): void { it('keeps demo relay gate-open queued but skips Shelly switch calls', function (): void {
$lane = new SelfserveLanePortControllerHarness( $lane = new SelfserveLanePortControllerHarness(
relayInId: 'demo-relay-in', relayInId: 'demo-relay-in',
@@ -235,6 +235,8 @@ it('wires self-serve lane gate open endpoint', function (): void {
expect($moduleSelfServeRoute)->toContain('selfserve_lane_port::ENTRANCE'); expect($moduleSelfServeRoute)->toContain('selfserve_lane_port::ENTRANCE');
expect($moduleSelfServeRoute)->toContain('selfserve_lane_port::EXIT'); expect($moduleSelfServeRoute)->toContain('selfserve_lane_port::EXIT');
expect($moduleSelfServeRoute)->toContain('$toggle_after = $this->requestedRelayToggleAfter(1);'); expect($moduleSelfServeRoute)->toContain('$toggle_after = $this->requestedRelayToggleAfter(1);');
expect($moduleSelfServeRoute)->toContain('MAX_GATE_OPEN_TOGGLE_AFTER_SECONDS = 5');
expect($moduleSelfServeRoute)->toContain('self::requireMaxValue($toggle_after, self::MAX_GATE_OPEN_TOGGLE_AFTER_SECONDS);');
expect($moduleSelfServeRoute)->toContain('$lane->open($gate, $toggle_after)'); expect($moduleSelfServeRoute)->toContain('$lane->open($gate, $toggle_after)');
expect($moduleSelfServeRoute)->toContain("'toggle_after' => \$toggle_after"); expect($moduleSelfServeRoute)->toContain("'toggle_after' => \$toggle_after");
expect($moduleSelfServeRoute)->toContain('private function requestedRelayToggleAfter'); expect($moduleSelfServeRoute)->toContain('private function requestedRelayToggleAfter');