Skip to content

Guard chunk string/template loaders against cyclic next_offset chains - #102

Open
memx13 wants to merge 2 commits into
williballenthin:masterfrom
memx13:fix/chunk-cyclic-offset-loop
Open

memx13 wants to merge 2 commits into
williballenthin:masterfrom
memx13:fix/chunk-cyclic-offset-loop

Conversation

@memx13

@memx13 memx13 commented Oct 23, 2026

Copy link
Copy Markdown

Fixes #101.

ChunkHeader._load_strings and ._load_templates follow each hash-bucket's on-disk next_offset chain with no protection against an offset that repeats. Since next_offset is read straight from the file, a crafted chunk with a cyclic chain (A.next_offset -> B, B.next_offset -> A) makes the while ofs > 0: loop run forever, so parsing an untrusted .evtx hangs the process (CWE-835).

This change records the offsets already visited while walking a bucket and breaks out of the loop when one repeats. Well-formed files follow a strictly terminating chain, so their behaviour is unchanged; the strings/templates dict is still keyed by offset exactly as before.

Test

tests/test_cyclic_offsets.py builds a single chunk whose string (and template) bucket points at a two-node cycle and asserts the loader terminates. The traversal runs in a short-lived daemon thread, so an unfixed loop surfaces as a failed assertion instead of wedging the whole suite.

Verified:

  • on current master both new tests fail (the loaders never return);
  • with this change pytest is green — 24 passed, including the 2 new tests;
  • black, isort and ruff (line-length 120) are clean on the changed files.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Infinite loop in ChunkHeader._load_strings / _load_templates on a crafted chunk (cyclic next_offset)

1 participant