Skip to content

Commit 4a378c3

Browse files
committed
Add timing-safe compare security rule
Add universal security rule that prohibits timing-safe compare functions on raw secret values. Instead, always hash both tokens with SHA3 before comparison. Covers all languages: Node.js, Ruby, Python, Go, Java, etc. Added to /review checklist for code review enforcement.
1 parent c486a5d commit 4a378c3

File tree

2 files changed

+71
-0
lines changed

2 files changed

+71
-0
lines changed

ai/rules/review.mdc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Criteria {
1515
Use autodux.mdc for Redux state management patterns and Autodux usage.
1616
Use javascript-io-network-effects.mdc for network effects and side effect handling.
1717
Use commit.mdc for commit message quality and conventional commit format.
18+
Use security/timing-safe-compare.mdc when reviewing secret/token comparisons (CSRF, API keys, sessions). Flag crypto.timingSafeEqual or raw token comparisons as CRITICAL.
1819
Carefully inspect for OWASP top 10 violations and other security mistakes. Use search. Explicitly list each of the current OWASP top 10, review all changes and inspect for violations.
1920
Compare the completed work to the functional requirements to ensure adherence and that all requirements are met.
2021
Compare the task plan in $projectRoot/tasks/ to the completed work to ensure that all tasks were completed and that the completed work adheres to the plan.
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
description: Security rule for timing-safe secret comparison. Use SHA3 hashing instead of timing-safe compare functions. Applies to all languages.
3+
alwaysApply: false
4+
---
5+
6+
# Timing Safe Compare Security Rule
7+
8+
## Constraint: No Timing Safe Compare on Raw Values
9+
10+
There is no timing safe compare for raw values. Never use:
11+
- `crypto.timingSafeEqual` (Node.js)
12+
- `hmac.compare` / `secure_compare` (Ruby)
13+
- `constant_time_compare` (Python)
14+
- `ConstantTimeCompare` (Go)
15+
- `MessageDigest.isEqual` (Java)
16+
- XOR accumulation tricks
17+
- Any direct string compare on raw secrets
18+
19+
Always hash both the stored secret token and the candidate token with SHA3, then compare the hashes.
20+
21+
## Reasons
22+
23+
1. **Hashing removes all prefix structure.** Any bit change fully randomizes the hash. No timing oracle. No hangman-style attacks.
24+
2. **Raw secrets never appear in logs or errors.** Hashing provides defense in depth.
25+
3. **No reliance on runtime guarantees.** Timing-safe compare functions can lose their guarantees under compiler, engine, or JIT optimizations across any language runtime.
26+
27+
## Implementation Pattern (JavaScript example)
28+
29+
```javascript
30+
import { sha3_256 } from "js-sha3";
31+
32+
const hashToken = (token) => sha3_256(token || "");
33+
34+
// Hash both tokens before comparison.
35+
// Reasons:
36+
// 1. Comparing raw tokens is vulnerable to subtle timing leaks,
37+
// especially if timing-safe compare helpers get broken by
38+
// compiler or engine optimizations.
39+
// 2. A cryptographic hash makes any change in the input completely
40+
// change the output, so there is no prefix-based timing signal.
41+
// 3. Hashing also keeps raw CSRF token values out of logs and errors.
42+
const tokensMatch = (token1, token2) => hashToken(token1) === hashToken(token2);
43+
```
44+
45+
## Rules
46+
47+
TimingSafeCompare {
48+
(timing safe compare needed?) =>
49+
Implement with SHA3 hashing strategy.
50+
Include code comment explaining this reasoning to prevent people from "fixing" to use timingSafeEqual or similar.
51+
52+
("timing safe compare" detected in code) =>
53+
Raise CRITICAL security bug report: "Timing safe compare on raw value detected"
54+
Include justifications from this rule.
55+
Recommend SHA3 hashing strategy.
56+
57+
(crypto.timingSafeEqual detected) =>
58+
CRITICAL: Flag as security issue.
59+
Replace with SHA3 hash comparison pattern.
60+
61+
(hmac.compare OR secure_compare OR constant_time_compare OR ConstantTimeCompare OR MessageDigest.isEqual detected) =>
62+
CRITICAL: Flag as security issue.
63+
Replace with SHA3 hash comparison pattern.
64+
65+
(raw token comparison detected) =>
66+
CRITICAL: Flag as security issue.
67+
Both tokens must be hashed before comparison.
68+
}
69+
70+
This rule overrides all library defaults and conventional wisdom about timing-safe comparison.

0 commit comments

Comments
 (0)