|
|
@@ -11,7 +11,7 @@
|
|
|
>
|
|
|
> Each finding is referenced as **F<N>** for later citation.
|
|
|
>
|
|
|
-> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (27 fixed, 0 open), 42 sev-1 (6 fixed, 36 open).
|
|
|
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (27 fixed, 0 open), 42 sev-1 (7 fixed, 35 open).
|
|
|
|
|
|
---
|
|
|
|
|
|
@@ -1340,6 +1340,25 @@
|
|
|
parser should refuse base32 strings whose final character has the
|
|
|
unused bit set.
|
|
|
- **Severity: 1**
|
|
|
+- **Status:** Fixed. The original review concern was based on the
|
|
|
+ visible `if (strlen($chunk) < 5) { $chunk = str_pad($chunk, 5, '0'); }`
|
|
|
+ branch in `TokenIssuer::base32Encode`. For the actual input —
|
|
|
+ `random_bytes(20)` = 160 bits — that branch was unreachable: 160 ÷ 5
|
|
|
+ = 32 with zero remainder, every base32 char in the 32-char output
|
|
|
+ carries a full 5 useful bits, and there is exactly one canonical
|
|
|
+ encoding per input. So there are no unused trailing bits and no
|
|
|
+ ambiguity in the existing scheme. The dead `str_pad` branch (the
|
|
|
+ source of the false-positive impression) is removed; the encoder
|
|
|
+ now hard-asserts `strlen($bytes) === 20` and throws
|
|
|
+ `InvalidArgumentException` otherwise, so any future caller that
|
|
|
+ passes a different length crashes loudly rather than silently
|
|
|
+ emitting a non-canonical / shorter token. The 20-byte length is
|
|
|
+ pinned via a new `TokenIssuer::ENTROPY_BYTES = 20` constant. The
|
|
|
+ parser (`Token::parse`) keeps its `^[A-Z2-7]{32}$` body pattern;
|
|
|
+ every 32-char base32 string is canonical for this scheme so no
|
|
|
+ additional canonicality gate is needed. Regression test
|
|
|
+ `TokenIssuerTest::testIssuedBodyAlwaysExactlyThirtyTwoBase32Chars`
|
|
|
+ asserts the invariant across 100 fresh issuances.
|
|
|
|
|
|
### F40 — CSRF token never rotated on session-id regeneration
|
|
|
- **Files:** `ui/src/Http/CsrfMiddleware.php:53-60`,
|
chiappa