fix: four critical security/DoS bugs (unsigned overflow, LZW stack overflow, WebP parser)#102
Conversation
Agent-Logs-Url: https://github.com/cross-org/image/sessions/239192e7-4f1a-46eb-8ab0-bca3089639c5 Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Addresses critical decoder crash/DoS vectors caused by signed 32-bit overflow in uint32 readers and by large-argument spreads in LZW/GIF parsing code paths.
Changes:
- Ensure 32-bit “unsigned” reads return unsigned values via
>>> 0inreadUint32LEand PNG big-endianreadUint32. - Replace
array.push(...typedArray)patterns with element-wise loops to avoidRangeErrorcall-stack/argument overflows during adversarial LZW/GIF decoding. - Document the security fixes in
CHANGELOG.mdand reformat the relevantTODO.mdentries.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Markdown line-wrapping / table formatting updates for tracked issues. |
| src/utils/lzw.ts | Prevent LZW output expansion from using spread-into-arguments. |
| src/utils/gif_decoder.ts | Avoid spreading sub-block bytes into blocks.push(...). |
| src/utils/byte_utils.ts | Make readUint32LE return a true unsigned 32-bit number (>>> 0). |
| src/formats/png_base.ts | Make big-endian uint32 reads return unsigned (>>> 0). |
| CHANGELOG.md | Add security-related “Fixed” entries describing the above changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot have a look at the ci failures |
…G formatting Agent-Logs-Url: https://github.com/cross-org/image/sessions/351e4626-a706-4f32-8731-2da70a607dbe Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Fixed in 8ac9b2e:
|
Four critical issues from TODO.md that allow crafted image files to crash or hang the decoder.
C1 & C2 — Signed 32-bit overflow in
readUint32LE/readUint32JS bitwise-OR always produces a signed 32-bit result. When bit 31 is set, the returned value is negative, breaking offset/length arithmetic across BMP, ICO, TIFF, and PNG decoders — e.g. a crafted PNG chunk length held negative causes an infinite parse loop (CPU DoS).
Fixed in both
src/utils/byte_utils.ts(LE) andsrc/formats/png_base.ts(BE).H4 — Duplicate signed
readUint32LEin WebP decodersrc/utils/webp_decoder.tsdefined its ownreadUint32LEthat still returned a signed 32-bit value. A crafted WebP with a high-bit chunk size could makepos + chunkSizego negative, silently short-circuiting the parse loop. The local copy has been removed and replaced with an import of the shared (now unsigned-safe)readUint32LEfromsrc/utils/byte_utils.ts.C3 — LZW call-stack overflow on adversarial GIF
output.push(...entry)spreads dictionary entries as function arguments. A crafted GIF with a long repeated run triggersRangeError: Maximum call stack size exceeded, crashing the runtime process. Fixed at three call sites insrc/utils/lzw.tsandsrc/utils/gif_decoder.ts: