Misc. vuln fixes#19
Conversation
|
Looks way to complicated to be correct =) |
Happy to break it down into separate commits if you like? Pretty confident the findings are correct. |
I went ahead and did this. Eyes appreciated. Given that embedded devices may use this library for firmware update, this presents a notable attack surface unless the device requires, for example, the delta patches be signed. |
|
If you allow anyone to upload delta firmware to your device, you probably have bigger problems as it seems your device is not protected by any authentication =) |
|
split into separate PRs and I might review some. and if possible, make corrupt patch check in helper functions to reduce the lines of code |
Summary
This PR fixes multiple vulnerabilities across the C apply path, the Python apply path, and the native Python extensions used during patch creation.
The common issue behind most of the real findings was that malformed patch metadata or caller-provided native buffers were not being rejected early enough. In practice that meant negative decoded sizes, impossible in-place patch geometry, oversized logical CRLE segments, and unchecked native buffers could reach arithmetic, reads, or writes that assumed the inputs were already valid.
This change hardens those boundaries. Valid patches should behave the same as before, but malformed or adversarial inputs now fail early and predictably instead of being normalized by casts, propagated into invalid geometry, or passed through to unsafe native code.
Breakdown
C library: negative decoded chunk sizes bypassed bounds validation
In
c/detools.c, decoded sizes from the patch stream could be negative and still pass throughcommon_process_size()after an implicit cast tosize_t.C library: in-place patches could encode
segment_size = 0In the in-place apply header,
segment_sizewas not rejected before being used in segment math.C library: in-place patches could encode
shift_size > memory_sizeThe in-place shift calculation assumed a valid layout and did not reject impossible memory geometry before using it.
Python apply path: negative sequential / in-place / bsdiff sizes were not rejected consistently
detools/apply.pytrusted several signed size fields from patch headers and chunk metadata longer than it should have.Python apply path: negative
from_sizecould reach file readsIn the in-place Python apply path, a negative decoded
from_sizecould becomeread(-N)behavior on file-like objects.Python apply path: short diff reads failed too late
During in-place segment application, a short source read could continue until
bsdiff.add_bytes()tripped on a length mismatch, leaving the failure mode too deep into the apply flow.CRLE decompressor buffered attacker-controlled logical output
In
detools/compression/crle.py, repeated and scattered segments could expand into large_outdatabuffers even when the caller only requested a small amount of output.bsdiffnative wrapper trusted caller-provided diff buffersdetools/bsdiff.cdid not verify that the scratch diff buffer was large enough forto_data.bsdiffnative wrapper trusted oversized inputsInputs larger than the algorithm’s
int32_tlimits could be truncated before reaching the core bsdiff logic.bsdiffnative wrapper trusted suffix-array contentsCaller-provided suffix-array entries were used as offsets without validating that they were in range.
suffix_arraynative wrapper did not validate output buffer sizingdetools.suffix_array.sais()anddivsufsort()assumed the caller had provided a large enough output buffer.bsdiff.add_bytes()leaked Python buffer exports on one error pathThe length-mismatch path returned without releasing acquired buffers.
Impact
Negative decoded chunk sizes in the C apply path
A crafted patch could bypass the intended size bound checks and drive invalid apply behavior in the low-level C patch path.
segment_size = 0in in-place C applyA crafted in-place patch could drive divide-by-zero style behavior and invalid segment processing.
shift_size > memory_sizein in-place C applyA crafted in-place patch could push the memory-shift logic into invalid geometry and nonsensical offsets.
Negative sizes in the Python apply path
Corrupt patches could slip past expected bounds checks and fail later in less predictable ways.
Negative
from_sizein Python in-place applyA crafted patch could trigger oversized reads from the source object, which is especially bad for constrained environments or unusual file-like wrappers.
Short reads in Python in-place diff application
Corrupt patches could fail mid-apply with a low-level mismatch instead of being rejected as invalid patch input up front.
CRLE output buffering
A crafted CRLE stream could force the decompressor to buffer far more logical output than the caller requested, creating an avoidable resource-exhaustion vector.
Unchecked diff scratch buffers in
bsdiffCallers of the native patch-creation helper could trigger out-of-bounds native writes with undersized buffers.
Oversized native inputs
Inputs above the underlying algorithm limits could truncate into invalid negative or wrapped values before reaching the search logic.
Unchecked suffix-array contents
Malformed suffix arrays could drive out-of-range native reads and undefined behavior in patch creation.
Unchecked suffix-array output buffers
Callers could hand the wrapper too-small output buffers and trigger native memory corruption.
Leaked buffer exports in
add_bytes()Repeated error-path use could accumulate Python-side resource leakage and leave mutable buffers pinned unexpectedly.
Testing
c/tst/test_fuzzer.c.tests/test_apply_security.pycovering:tests/test_crle.pyto verify repeated and scattered segments now decompress incrementally instead of over-buffering.tests/test_bsdiff.pycovering:INT32_MAXadd_bytes()buffer-release error pathtests/test_suffix_array.pycovering:INT32_MAX