Skip to content

Commit 44c20c4

Browse files
sirtimidclaude
andcommitted
fix(transport): use atomic checkAndRecord to prevent TOCTOU race
Revert to using checkAndRecord() for message rate limiting instead of separate check and record calls. The separated approach had a TOCTOU race where concurrent sends could all pass the check before any recorded, bypassing the rate limit. Yes, failed sends now consume quota, but this is necessary for security - recording after send would allow attackers to make unlimited concurrent attempts that bypass the rate limit. Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 1391562 commit 44c20c4

File tree

1 file changed

+5
-17
lines changed

1 file changed

+5
-17
lines changed

packages/ocap-kernel/src/remotes/platform/transport.ts

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
DEFAULT_MAX_CONCURRENT_CONNECTIONS,
1212
DEFAULT_MAX_MESSAGE_SIZE_BYTES,
1313
DEFAULT_MESSAGE_RATE_LIMIT,
14-
DEFAULT_MESSAGE_RATE_WINDOW_MS,
1514
DEFAULT_STALE_PEER_TIMEOUT_MS,
1615
DEFAULT_WRITE_TIMEOUT_MS,
1716
SCTP_USER_INITIATED_ABORT,
@@ -348,20 +347,11 @@ export async function initTransport(
348347
// Validate message size before sending
349348
validateMessageSize(message);
350349

351-
// Check message rate limit (check only, record after successful send)
352-
if (messageRateLimiter.wouldExceedLimit(targetPeerId)) {
353-
const currentCount = messageRateLimiter.getCurrentCount(targetPeerId);
354-
throw new ResourceLimitError(
355-
`Rate limit exceeded: ${currentCount}/${maxMessagesPerSecond} messageRate in ${DEFAULT_MESSAGE_RATE_WINDOW_MS}ms window`,
356-
{
357-
data: {
358-
limitType: 'messageRate',
359-
current: currentCount,
360-
limit: maxMessagesPerSecond,
361-
},
362-
},
363-
);
364-
}
350+
// Check and record message rate limit atomically to prevent TOCTOU race
351+
// Note: This records before send completes, so failed sends consume quota.
352+
// This is intentional - recording after send would allow concurrent sends
353+
// to bypass the rate limit via TOCTOU attacks.
354+
messageRateLimiter.checkAndRecord(targetPeerId, 'messageRate');
365355

366356
const state = peerStateManager.getState(targetPeerId);
367357

@@ -415,8 +405,6 @@ export async function initTransport(
415405
fromString(message),
416406
DEFAULT_WRITE_TIMEOUT_MS,
417407
);
418-
// Record message rate only after successful send
419-
messageRateLimiter.recordEvent(targetPeerId);
420408
peerStateManager.updateConnectionTime(targetPeerId);
421409
reconnectionManager.resetBackoff(targetPeerId);
422410
} catch (problem) {

0 commit comments

Comments
 (0)