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
The issue's line numbers are out of date. In src/metadata-backup.ts today:
extractExifFromJpeg is at line 21, with the unchecked Buffer.from at 38 and the offset += 2 + len step at 45.
The bare catch blocks are at 65, 73, 94 and 110.
Definition of done: as in the issue body. For item 3, record the failure reason as a field in the per-file JSON, following that JSON's existing field naming. Put the crafted JPEG inputs directly in the test file as short byte arrays; add no binary fixtures. The file-name sanitizing at the top of the file is #9 work and stays as it is.
Model: opus-5-5
## Implementer brief (branch `next2`, after https://git.eeqj.de/sneak/quak/pulls/78)
The issue's line numbers are out of date. In `src/metadata-backup.ts` today:
- `extractExifFromJpeg` is at line 21, with the unchecked `Buffer.from` at 38 and the `offset += 2 + len` step at 45.
- The bare `catch` blocks are at 65, 73, 94 and 110.
Definition of done: as in the issue body. For item 3, record the failure reason as a field in the per-file JSON, following that JSON's existing field naming. Put the crafted JPEG inputs directly in the test file as short byte arrays; add no binary fixtures. The file-name sanitizing at the top of the file is https://git.eeqj.de/sneak/quak/issues/9 work and stays as it is.
Model: opus-5-5
Implemented in #84. Segment lengths are now bounds-checked, so the scan always ends. Per-file JSON records imageMetadata.exifError for a malformed or unparseable EXIF segment and imageMetadataError when the original cannot be read.
Model: opus-5-5
Implemented in https://git.eeqj.de/sneak/quak/pulls/84. Segment lengths are now bounds-checked, so the scan always ends. Per-file JSON records `imageMetadata.exifError` for a malformed or unparseable EXIF segment and `imageMetadataError` when the original cannot be read.
Model: opus-5-5
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.Implementer brief (branch
next2, after #78)The issue's line numbers are out of date. In
src/metadata-backup.tstoday:extractExifFromJpegis at line 21, with the uncheckedBuffer.fromat 38 and theoffset += 2 + lenstep at 45.catchblocks are at 65, 73, 94 and 110.Definition of done: as in the issue body. For item 3, record the failure reason as a field in the per-file JSON, following that JSON's existing field naming. Put the crafted JPEG inputs directly in the test file as short byte arrays; add no binary fixtures. The file-name sanitizing at the top of the file is #9 work and stays as it is.
Model: opus-5-5
Implemented in #84. Segment lengths are now bounds-checked, so the scan always ends. Per-file JSON records
imageMetadata.exifErrorfor a malformed or unparseable EXIF segment andimageMetadataErrorwhen the original cannot be read.Model: opus-5-5