A size check is written as `if (offset + length > buffer_size) reject;` where both `offset` and `length` come from an untrusted request. Explain why this check is unsafe on fixed-width integers and how you would rewrite it so it is correct for the whole input range.
answer
- the guard contains the overflow
- offset > size, then length > size - offset
- order matters — subtract only after the first bound
- checked add > hand-proved rearrangement
- `a + b < a` is UB for signed types
basics
~20 sThe check itself does the overflowing arithmetic: if offset + length wraps, the sum becomes small and the guard passes for values that are far out of range. Rewrite it so nothing can wrap — compare by subtraction against the known limit, or use a checked-addition operation that fails on overflow.
solid answer
~50 sThe guard evaluates the same unsafe expression it is meant to protect. With attacker-chosen `offset` and `length` near the type's maximum, `offset + length` wraps to a small residue, the comparison succeeds, and the caller then reads or writes using the *unwrapped* operands. Two correct rewrites: - **Subtract instead of add**, in an order that cannot wrap: reject if `offset > buffer_size`, then reject if `length > buffer_size - offset`. Every intermediate stays inside the range. - **Use checked arithmetic**: an add that returns failure (or traps) on overflow, then compare the verified sum. Avoid the C idiom `if (offset + length < offset)`: for *signed* types that is undefined behaviour and an optimiser may delete it. Validate each field's own range at the parse boundary as well, but do not rely on that alone — per-field bounds are only sound if they provably bound the whole expression.
go deeper
Say the guard's own addition can wrap so the comparison passes; show the subtract-after-bounding rewrite and mention checked addition.
Add the ordering argument for the subtraction, the signed-UB reason not to use the self-check idiom, and the signedness/truncation variants.
Push for the checked-arithmetic helper so the proof lives in one place, and treat declared lengths in protocols as claims to be bounded by bytes actually held.
Frame it as an API question: which types and functions in the codebase are permitted to do arithmetic on untrusted sizes, so the unsafe form is hard to write rather than found by review.
## Why the guard is the bug The intent is: "the requested window must lie inside the buffer." The code expresses that as `offset + length > buffer_size`. On fixed-width integers the left-hand side is not the mathematical sum; it is the sum modulo 2^n. Choose `offset` and `length` so their true sum exceeds the type's maximum, and the computed value drops to a tiny residue. The comparison then answers a question about a number that does not exist anywhere in the program's intent, returns "in range", and the code proceeds to use the original, un-wrapped `offset` and `length` in a copy, a read, a seek or an allocation. This is the general shape of the defect: **the check and the use disagree because the check contains the arithmetic**. It appears identically in a quota test (`used + requested > limit`), a pagination test (`page * page_size`), and an allocation (`count * element_size`). ## Rewrite 1 — rearrange so nothing can wrap ``` if (offset > buffer_size) reject; // no arithmetic yet if (length > buffer_size - offset) reject; // subtraction is now safe ``` The first line establishes `offset <= buffer_size`, which makes `buffer_size - offset` non-negative and bounded by `buffer_size`. Every intermediate value stays inside the representable range, so the comparison is a genuine proof over the whole input domain. Order matters: swap the lines and the subtraction underflows to a huge unsigned value (or a negative signed one) and the guard passes again. ## Rewrite 2 — checked arithmetic Better where the language offers it: an addition that reports overflow rather than wrapping (a checked/overflow-aware add, or a language mode that traps). The code then reads as "compute the end; if that failed, reject; otherwise compare". This is the **structural separation** rung of the defence ladder — the wrong value cannot reach the comparison at all — whereas the rearranged subtraction is a hand-proved instance that a future edit can invalidate. Saturating arithmetic is *not* a substitute here: clamping to the maximum happens to be conservative for this comparison, but the same habit applied to an allocation size turns a wrap into a maximal allocation. ## The idiom to avoid `if (offset + length < offset) reject;` is the classic self-check. For unsigned types it is well defined and works. For **signed** types the overflow is undefined behaviour, so a compiler is entitled to reason "signed overflow never happens, therefore `offset + length` is never less than `offset`, therefore this branch is dead" and remove it. The defence disappears silently at optimisation, which is worse than never writing it, because the code reads as protected. ## Related traps in the same expression family - **Signedness**: if `length` is signed and the copy takes an unsigned size, a negative `length` passes a naive `length > buffer_size` test and converts to a near-maximal size. - **Truncation**: parsing a 64-bit declared length into a 32-bit variable discards the high bits, so a huge declared length becomes a small accepted one — and the code that later reads the stream still uses the full value. - **Zero and empty**: a length of zero often skips the loop but may still be used to index or to size an allocation; decide explicitly. - **Declared vs actual**: in a length-prefixed protocol or file format, the declared length is a claim, not a fact. Bound it by the number of bytes you actually hold, and never allocate the declared amount before you have. ## How to make it stick Do not scatter hand-proved comparisons. Put the operation behind one small helper — `checked_add`, `range_within(buffer_size, offset, length)` — so the proof exists once and every call site inherits it. Then the review question is mechanical: does this arithmetic on untrusted numbers go through the helper? That is a far cheaper question than re-deriving the algebra at every call site, and it survives the refactor where someone adds a third term to the sum.
- You also validate `offset` and `length` individually when parsing the request. Does that remove the need for the combined check?Only if the per-field caps provably bound the whole expression — for example both capped at buffer_size/2, or at values whose sum cannot reach the type's maximum. That reasoning is easy to break: someone raises one cap, or adds a third term, and the proof silently lapses. Keep per-field validation as defence in depth and still perform the combined check where the value is used.
- Same idea, but the code computes an allocation as `count * element_size`. What changes?Multiplication overflows far sooner, and the consequence is worse: the wrap yields a small allocation while the subsequent loop still runs `count` times, so an undersized buffer is written past its end. Use a checked multiply, or bound `count` by `MAX / element_size` before multiplying — and never saturate an allocation size, since clamping to the maximum turns the wrap into an enormous allocation.
saying these in an interview costs you the question
- "The comparison is fine, it's the copy that's wrong" — the comparison is where the overflow happens.
- Rewriting as `offset + length < offset` on signed types and calling it fixed.
- Doing `buffer_size - offset` before proving `offset <= buffer_size`, which just moves the wrap to an underflow.
- Assuming a per-field range check at parse time makes the combined arithmetic safe.
- Believing a runtime will throw on overflow — most silently wrap.