extractExifFromJpeg in src/metadata-backup.ts:79-100 walks JPEG segment markers using
length fields taken directly from the file. Two problems:
Buffer.from(buf.buffer, byteOffset + offset + 4, len - 2) is constructed from an
unvalidated len, so a segment length that runs past the end of the buffer throws RangeError. The outer catch swallows it, so the file silently backs up with no metadata
and no indication that the input was malformed.
A segment with len === 0 makes offset += 2 + len advance by 2 forever, so the scan
cannot terminate on certain inputs.
The bytes come from decrypted user photos, so the input is not hostile in the normal case —
but a corrupt or truncated original in someone's library should not be able to hang quak backup-metadata --exif, and a parse failure should be visible rather than
indistinguishable from "this photo has no EXIF".
The same file has four more bare catch { } blocks (:118-120, :125-128, :147-149, :162-163) that flatten every failure to undefined.
Definition of done
Every segment length read from the file is bounds-checked against the remaining buffer
before use; an out-of-range length ends the scan cleanly rather than throwing.
The scan is guaranteed to terminate on any input, including zero and undersized segment
lengths. A test proves this with a crafted input.
Metadata extraction failures are distinguishable from "no metadata present": the per-file
JSON records that extraction was attempted and failed, with a reason, rather than silently
omitting the field.
The remaining bare catch { } blocks in src/metadata-backup.ts either record a reason or
carry a comment explaining why swallowing is correct there.
Tests cover: a truncated JPEG, a JPEG with a zero-length segment, a JPEG with a segment
length past end-of-buffer, a valid JPEG with EXIF (unchanged behaviour), and a file that is
not a JPEG at all.
make check green.
TODO.md updated in the same commit.
## Problem
`extractExifFromJpeg` in `src/metadata-backup.ts:79-100` walks JPEG segment markers using
length fields taken directly from the file. Two problems:
- `Buffer.from(buf.buffer, byteOffset + offset + 4, len - 2)` is constructed from an
unvalidated `len`, so a segment length that runs past the end of the buffer throws
`RangeError`. The outer `catch` swallows it, so the file silently backs up with no metadata
and no indication that the input was malformed.
- A segment with `len === 0` makes `offset += 2 + len` advance by 2 forever, so the scan
cannot terminate on certain inputs.
The bytes come from decrypted user photos, so the input is not hostile in the normal case —
but a corrupt or truncated original in someone's library should not be able to hang
`quak backup-metadata --exif`, and a parse failure should be visible rather than
indistinguishable from "this photo has no EXIF".
The same file has four more bare `catch { }` blocks (`:118-120`, `:125-128`, `:147-149`,
`:162-163`) that flatten every failure to `undefined`.
## Definition of done
1. Every segment length read from the file is bounds-checked against the remaining buffer
before use; an out-of-range length ends the scan cleanly rather than throwing.
2. The scan is guaranteed to terminate on any input, including zero and undersized segment
lengths. A test proves this with a crafted input.
3. Metadata extraction failures are distinguishable from "no metadata present": the per-file
JSON records that extraction was attempted and failed, with a reason, rather than silently
omitting the field.
4. The remaining bare `catch { }` blocks in `src/metadata-backup.ts` either record a reason or
carry a comment explaining why swallowing is correct there.
5. Tests cover: a truncated JPEG, a JPEG with a zero-length segment, a JPEG with a segment
length past end-of-buffer, a valid JPEG with EXIF (unchanged behaviour), and a file that is
not a JPEG at all.
6. `make check` green.
7. `TODO.md` updated in the same commit.
clawbot
added this to the 1.0.0 milestone 2026-08-09 03:45:13 +02:00
clawbot
self-assigned this 2026-08-09 03:45:13 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
extractExifFromJpeginsrc/metadata-backup.ts:79-100walks JPEG segment markers usinglength fields taken directly from the file. Two problems:
Buffer.from(buf.buffer, byteOffset + offset + 4, len - 2)is constructed from anunvalidated
len, so a segment length that runs past the end of the buffer throwsRangeError. The outercatchswallows it, so the file silently backs up with no metadataand no indication that the input was malformed.
len === 0makesoffset += 2 + lenadvance by 2 forever, so the scancannot terminate on certain inputs.
The bytes come from decrypted user photos, so the input is not hostile in the normal case —
but a corrupt or truncated original in someone's library should not be able to hang
quak backup-metadata --exif, and a parse failure should be visible rather thanindistinguishable from "this photo has no EXIF".
The same file has four more bare
catch { }blocks (:118-120,:125-128,:147-149,:162-163) that flatten every failure toundefined.Definition of done
before use; an out-of-range length ends the scan cleanly rather than throwing.
lengths. A test proves this with a crafted input.
JSON records that extraction was attempted and failed, with a reason, rather than silently
omitting the field.
catch { }blocks insrc/metadata-backup.tseither record a reason orcarry a comment explaining why swallowing is correct there.
length past end-of-buffer, a valid JPEG with EXIF (unchanged behaviour), and a file that is
not a JPEG at all.
make checkgreen.TODO.mdupdated in the same commit.