es-ude / es-ude/OnDeviceTraining
serial: unchecked element-count product in deserializeTensor/skipSerializedTensor — crafted dims can wrap and mis-seek
- Dominant language
- C
- Stars
- 1
- Forks
- 3
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 8
Description
## Problem
Found in the #380 PR3 final review (pre-existing pattern, extended by the new skip path):
- `calcNumberOfBytesForData` multiplies the dims product unchecked (`src/tensor/Tensor.c` sizing chain).
- `skipSerializedTensor` (`src/serial/Deserialize.c`, #380 PR3) reads up to 8 attacker-controlled `u32` dims; a crafted combination can wrap `numberOfElements`, and a payload size landing in `[2^63, 2^64)` becomes a **negative** `long` in `fseek(..., SEEK_CUR)` — a *backward* seek that succeeds (position stays ≥ 0), which the SEEK_END truncation guard cannot catch (`recordEnd < fileSize` passes).
- `deserializeTensor` shares the unchecked arithmetic but additionally benefits from the skeleton cross-checks (tag/rank/dtype/#316 payload comparison), which the skip path inherently lacks.
Blast radius (assessed in review): pure stream desync — every subsequent record read is still gated by skeleton cross-checks, so **no OOB write is reachable**. Robustness/parse-integrity issue, not memory safety.
## Fix sketch
- Overflow-checked product helper (fail-fast on wrap) used by both `deserializeTensor`'s sizing path and `skipSerializedTensor`; reject payload sizes that don't fit a `long` before any `fseek`.
- Optionally a sane global element-count cap for serialized records (MCU-realistic bound) as a cheaper second line.
- Regression tests: crafted-dims record → clean exit(1), both in the skip path and the normal path.
Contributor guide
Research direction
Start with calcNumberOfBytesForData in src/tensor/Tensor.c and skipSerializedTensor plus deserializeTensor in src/serial/Deserialize.c, then trace how serialized records are tested. Add regression coverage for crafted dimensions in both skip and normal paths; done means each malformed record exits cleanly with exit(1) without mis-seeking.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100