es-ude / es-ude/OnDeviceTraining

serial: unchecked element-count product in deserializeTensor/skipSerializedTensor — crafted dims can wrap and mis-seek

Open
#394 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.