fix: begrens het lezen van beeld-sidecars #542

Merged

brenno merged 1 commit from feat/beveiligingsharding into main 2026-07-22 11:38:56 +00:00

Conversation 0 Commits 1 Files changed 2 +102

brenno commented 2026-07-22 11:38:54 +00:00

Owner

Copy link

Summary

The first of the nine items in #516: the unbounded sidecar reads. Partial — #516 stays open.

What was wrong

All three read paths in ImageSidecarStore read the file with no size check. A sidecar travels with a deck, so it can arrive from a package, a repository, or somebody else's folder — jsonDecode then puts a second copy on top of the first.

The write path was the sharper one. Without a cap, write() carried on with an empty map, wrote that over the file, and every other caption in that folder was gone. That is precisely the failure the existing catch block next to it describes and defends against — reached by a different route.

The cap

1 MiB. The file is a flat map of filename to caption, so that is room for thousands of images. The excess is logged rather than swallowed: silently skipping makes the user believe their captions are gone when they are still on disk.

Verification

Three tests, including that a too-large sidecar is left byte-identical after a write attempt. Mutation-checked: removing the cap makes two of the three fail.

make check green.

Still open in #516

The other eight items, of which two deserve a note now:

• audienceBoundary generic — I measured this before attempting it. There are 208 raw Deck/List<Slide> parameters across 67 files outside privacy/, so the obvious inversion ("baseline every Deck-taking function") produces noise rather than signal. A workable generic rule has to key on output — a function that takes a raw deck and reaches an output primitive — which is a real design, not a quick fix. Worth doing, not worth guessing at.
• PBKDF2-SHA1 at 1000 iterations is already recorded as a deliberate deviation in assurance/ASVS-5.0.0-afwijkingen.md with its reasoning. It should be resolved there if it is resolved, not re-decided in a hardening pass.

## Summary The first of the nine items in #516: the unbounded sidecar reads. Partial — #516 stays open. ## What was wrong All three read paths in `ImageSidecarStore` read the file with no size check. A sidecar travels with a deck, so it can arrive from a package, a repository, or somebody else's folder — `jsonDecode` then puts a second copy on top of the first. **The write path was the sharper one.** Without a cap, `write()` carried on with an empty map, wrote that over the file, and every other caption in that folder was gone. That is precisely the failure the existing `catch` block next to it describes and defends against — reached by a different route. ## The cap 1 MiB. The file is a flat map of filename to caption, so that is room for thousands of images. The excess is logged rather than swallowed: silently skipping makes the user believe their captions are gone when they are still on disk. ## Verification Three tests, including that a too-large sidecar is left **byte-identical** after a write attempt. **Mutation-checked:** removing the cap makes two of the three fail. `make check` green. ## Still open in #516 The other eight items, of which two deserve a note now: - **`audienceBoundary` generic** — I measured this before attempting it. There are 208 raw `Deck`/`List<Slide>` parameters across 67 files outside `privacy/`, so the obvious inversion ("baseline every Deck-taking function") produces noise rather than signal. A workable generic rule has to key on *output* — a function that takes a raw deck **and** reaches an output primitive — which is a real design, not a quick fix. Worth doing, not worth guessing at. - **PBKDF2-SHA1 at 1000 iterations** is already recorded as a deliberate deviation in `assurance/ASVS-5.0.0-afwijkingen.md` with its reasoning. It should be resolved *there* if it is resolved, not re-decided in a hardening pass.

brenno added 1 commit 2026-07-22 11:38:54 +00:00

fix(beveiliging): begrens het lezen van beeld-sidecars ... 5c9fea019b

Een sidecar reist mee met een deck — uit een pakket, een repo, of een map
van iemand anders. Alle drie de leespaden in ImageSidecarStore lazen hem
onbegrensd in, en jsonDecode legt er nog een kopie bovenop.

De schrijfkant was de scherpste. Zonder grens ging write() door met een
lege map, schreef die over het bestand heen, en waren alle andere
bijschriften in die map weg — precies de fout die het catch-blok ernaast
beschrijft, alleen met een andere aanleiding.

Eén MiB is ruim voor duizenden afbeeldingen; het gaat om een platte map
van bestandsnaam naar bijschrift. Het overschot wordt gemeld, want stil
overslaan laat de gebruiker denken dat zijn bijschriften weg zijn terwijl
ze er nog staan.

Getoetst met een mutatie: de grens weghalen laat twee van de drie tests
vallen.

Onderdeel van #516.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

brenno merged commit cde5bc0969 into main 2026-07-22 11:38:56 +00:00

brenno deleted branch feat/beveiligingsharding 2026-07-22 11:38:56 +00:00

brenno referenced this pull request from a commit 2026-07-22 11:38:56 +00:00

Merge pull request 'fix: begrens het lezen van beeld-sidecars' (#542) from feat/beveiligingsharding into main

brenno referenced this pull request 2026-07-22 11:47:10 +00:00

feat: versie op één plek, zichtbaar in de app, en com.example eruit #544

brenno referenced this pull request 2026-07-22 12:05:45 +00:00

[Feature] Security hardening: nine items below the blocking layer #516

brenno referenced this pull request 2026-07-22 14:28:29 +00:00

fix: zet vijf punten uit de beveiligingsronde recht (#516) #560

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

accepted

Assessed and will be built

  

bug

Something is broken

  

declined

Assessed and will not be built

  

docs

Documentation only

  

duplicate

Already tracked elsewhere

  

enhancement

New feature or improvement request

  

good first issue

Small, self-contained, and a good place to start

  

in-progress

Someone is working on this right now. Do not start it in parallel.

  

needs-info

Waiting for the reporter

  

privacy

Personal data can leak. Handled before anything else.

  

security

A weakness someone could use. Handled before anything else.

  

triage

Awaiting assessment

No labels

accepted

bug

declined

docs

duplicate

enhancement

good first issue

in-progress

needs-info

privacy

security

triage

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

LibreKAT/Ocideck!542

No description provided.