fix: handle negative node read size in ubifs walk#123
Open
urknall wants to merge 1 commit intoonekey-sec:mainfrom
Open
fix: handle negative node read size in ubifs walk#123urknall wants to merge 1 commit intoonekey-sec:mainfrom
urknall wants to merge 1 commit intoonekey-sec:mainfrom
Conversation
In ubi_reader ≤0.8.13 (master as of 2026-03), _index() computes
read_size = chdr.len - UBIFS_COMMON_HDR_SZ
node_buf = ubifs.file.read(read_size)
without first checking that read_size >= 0. A zero-filled erase block
(factory bad block, or any block whose first bytes are all 0x00) causes
the common-node header to be read as all-zeros, so chdr.len == 0 and
read_size == -24 (== -UBIFS_COMMON_HDR_SZ). Python's io.BufferedReader
raises
ValueError: read length must be non-negative or -1
which propagates through walk.index() and is caught by extract_files as:
extract_files Error: read length must be non-negative or -1
This aborts the entire UBIFS extraction even though only one LEB is
affected and the rest of the filesystem is intact.
Fix: add a bounds check for read_size < 0 immediately after the
subtraction, matching the style of the adjacent len(buf) guard that
already handles the case where the common header itself is short:
- With -w / --warn-only-block-read-errors the corrupted node is skipped
with an 'Error'-level message and _index() returns.
- Without -w the existing 'Fatal' path is taken, preserving the pre-fix
behaviour for strict mode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In ubi_reader ≤0.8.13 (master as of 2026-03), _index() computes
without first checking that read_size >= 0. A zero-filled erase block (factory bad block, or any block whose first bytes are all 0x00) causes the common-node header to be read as all-zeros, so chdr.len == 0 and read_size == -24 (== -UBIFS_COMMON_HDR_SZ). Python's io.BufferedReader raises
which propagates through walk.index() and is caught by extract_files as:
This aborts the entire UBIFS extraction even though only one LEB is affected and the rest of the filesystem is intact.
Fix: add a bounds check for read_size < 0 immediately after the subtraction, matching the style of the adjacent len(buf) guard that already handles the case where the common header itself is short: