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

Merged
brenno merged 5 commits from sec/sidecar-leesgrens into main 2026-07-22 14:28:37 +00:00
Owner

Vijf van de negen punten uit #516, in de volgorde die de regel voorschrijft.

Wat erin zit

Onbegrensde leesbewerkingen (twee van de drie plekken). De vier sidecars
naast een deck — .ink.json, .user-notes.json, .miauw.json, .seal.json
werden ingelezen hoe groot ze ook waren, met de kopie die jsonDecode er
bovenop legt. De schrijfkant was de scherpste helft, net als bij #542: wat niet
is ingelezen mag ook niet vervangen of verwijderd worden, anders wist opslaan
de strepen die deze build alleen maar had overgeslagen. Die redenering stond er
al voor "uit een nieuwere build", dus lopen beide aanleidingen nu door één
poort — wat vier keer hetzelfde logblok scheelt.

Bij het CVE-bulkarchief gold hetzelfde één deur eerder: niet alleen het
uitpakken van de geneste zip was onbegrensd, ook de download zelf. Beide toetsen
nu de aankondiging én de werkelijk verwerkte bytes — de eerste is gratis, de
tweede is wat een liegende header opvangt.

De derde plek uit de issue, de grafiekdata, bleek al begrensd
(maxChartDataBytes); de issuetekst was daar verouderd.

network_sink_guard_test voor Process.start en WebViewController. Bij
allebei opent iets anders dan deze app de socket — git doet zijn eigen DNS, de
webview-engine ook — dus safeResolve en socket-pinning bestaan daar niet, en
ze stonden daarom in het geheel niet in dit bestand. Bij disk_traces.dart stond
de reden al opgeschreven: de semgrep-uitzondering zit daar bewust op de régel,
"een tweede subproces hier moet wél alarm geven". Alleen zit semgrep niet in
make check, dus dat alarm bestond nergens.

sanitize_svg van deny-list naar allow-list. Het bezwaar dat in dat bestand
stond was echt: een allow-list op gevoel laat de eerste de beste
sequentiediagram half renderen zónder foutmelding, en stil verlies is erger dan
een bekend gat. Maar de lijst hoeft niet geraden te worden. flutter_svg is de
enige lezer die deze SVG ooit krijgt, en de parser erachter
(vector_graphics_compiler) noemt zijn woordenschat expliciet. Alles daarbuiten
gooit die parser zélf al weg, dus de lijst is nooit strenger dan de lezer — en
dat draait de ruil om.

Dat het uitmaakt bleek meteen: x1/y1/x2/y2 ontbraken in de eerste versie van
de lijst, waardoor elke <line> een punt werd. De toets kijkt daarom naar de
attributen en niet alleen naar de elementnamen.

Regressietests

Elke reparatie heeft er een, en elke test is één keer rood gezien tegen de
ónherstelde code:

  • deck_sidecar_cap_test.dart — lezen én de schrijfkant die het bestand met rust
    laat.
  • cve_bulk_ingest_test.dart — een te grote aankondiging, en een zip die over
    zijn eigen omvang liegt (kopvelden gepatcht) zodat de tweede toets ook echt
    aan de beurt komt.
  • cve_download_cap_test.dart — apart, en dat is met opzet: download pint de
    socket vast via NetGuard, en die weigert loopback. Een testserver op deze
    machine is per ontwerp onbereikbaar, dus de grens staat los in streamCapped,
    juist zodat hij te bereiken is.
  • sanitize_svg_test.dart — de hele getekende woordenschat, op attribuutniveau.
  • network_sink_guard_test.dart — getoetst met een geplante uitgang, in beide
    richtingen.

Wat dit oplevert en wat het kost

De bewaker-blik levert één echte botsing op, en die blijft staan: een gebruiker
met een sidecar boven de grens ziet zijn strepen niet terug, en hoort dat alleen
via het logbestand. Veiligheid wint hier van zichtbaarheid, maar niet gratis —
het verlies is wél omkeerbaar (het bestand blijft onaangeroerd staan, dus het is
te openen met ander gereedschap of te verkleinen), en de grens van 16 MiB komt
bij echt tekenwerk neer op uren aaneengesloten annoteren.

Het zichtbaar maken vraagt een eigen waarschuwingskanaal plus 31 vertalingen —
het bestaande kanaal is ChartDataWarning en zou de verkeerde zin tonen. Dat is
eigen werk met een eigen afweging, geen aanhangsel van een beveiligingsgrens, en
gaat als aparte issue de lijst op.

Poorten

make check groen (5622 tests, dekking 86,2%). make check-secrets schoon
(gitleaks over 1602 commits, trufflehog over werkboom én historie).
make sast schoon (semgrep, 3 Dart-regels over 669 bestanden, 0 bevindingen).
DAST niet gedraaid: ZAP is een webapp-scanner en de webbundel is CanvasKit — er
valt niets te spideren, en deze wijziging raakt het geserveerde oppervlak niet.

De klasseratchet viel onderweg om (FileService, 2938 tegen plafond 2904). De
twee sidecar-helpers raken geen enkel veld van die klasse en staan nu buiten de
klasse; de winst is vastgezet op 2885 in plaats van het plafond te verhogen.

Refs #516

Vijf van de negen punten uit #516, in de volgorde die de regel voorschrijft. ## Wat erin zit **Onbegrensde leesbewerkingen (twee van de drie plekken).** De vier sidecars naast een deck — `.ink.json`, `.user-notes.json`, `.miauw.json`, `.seal.json` — werden ingelezen hoe groot ze ook waren, met de kopie die `jsonDecode` er bovenop legt. De schrijfkant was de scherpste helft, net als bij #542: wat niet is ingelezen mag ook niet vervangen of verwijderd worden, anders wist opslaan de strepen die deze build alleen maar had overgeslagen. Die redenering stond er al voor "uit een nieuwere build", dus lopen beide aanleidingen nu door één poort — wat vier keer hetzelfde logblok scheelt. Bij het CVE-bulkarchief gold hetzelfde één deur eerder: niet alleen het uitpakken van de geneste zip was onbegrensd, ook de download zelf. Beide toetsen nu de aankondiging én de werkelijk verwerkte bytes — de eerste is gratis, de tweede is wat een liegende header opvangt. De derde plek uit de issue, de grafiekdata, bleek al begrensd (`maxChartDataBytes`); de issuetekst was daar verouderd. **`network_sink_guard_test` voor `Process.start` en `WebViewController`.** Bij allebei opent iets anders dan deze app de socket — `git` doet zijn eigen DNS, de webview-engine ook — dus `safeResolve` en socket-pinning bestaan daar niet, en ze stonden daarom in het geheel niet in dit bestand. Bij `disk_traces.dart` stond de reden al opgeschreven: de semgrep-uitzondering zit daar bewust op de régel, "een tweede subproces hier moet wél alarm geven". Alleen zit semgrep niet in `make check`, dus dat alarm bestond nergens. **`sanitize_svg` van deny-list naar allow-list.** Het bezwaar dat in dat bestand stond was echt: een allow-list op gevoel laat de eerste de beste sequentiediagram half renderen zónder foutmelding, en stil verlies is erger dan een bekend gat. Maar de lijst hoeft niet geraden te worden. `flutter_svg` is de enige lezer die deze SVG ooit krijgt, en de parser erachter (`vector_graphics_compiler`) noemt zijn woordenschat expliciet. Alles daarbuiten gooit die parser zélf al weg, dus de lijst is nooit strenger dan de lezer — en dat draait de ruil om. Dat het uitmaakt bleek meteen: `x1/y1/x2/y2` ontbraken in de eerste versie van de lijst, waardoor elke `<line>` een punt werd. De toets kijkt daarom naar de attributen en niet alleen naar de elementnamen. ## Regressietests Elke reparatie heeft er een, en elke test is één keer rood gezien tegen de ónherstelde code: - `deck_sidecar_cap_test.dart` — lezen én de schrijfkant die het bestand met rust laat. - `cve_bulk_ingest_test.dart` — een te grote aankondiging, en een zip die over zijn eigen omvang liegt (kopvelden gepatcht) zodat de tweede toets ook echt aan de beurt komt. - `cve_download_cap_test.dart` — apart, en dat is met opzet: `download` pint de socket vast via NetGuard, en die weigert loopback. Een testserver op deze machine is per ontwerp onbereikbaar, dus de grens staat los in `streamCapped`, juist zodat hij te bereiken is. - `sanitize_svg_test.dart` — de hele getekende woordenschat, op attribuutniveau. - `network_sink_guard_test.dart` — getoetst met een geplante uitgang, in beide richtingen. ## Wat dit oplevert en wat het kost De bewaker-blik levert één echte botsing op, en die blijft staan: een gebruiker met een sidecar boven de grens ziet zijn strepen niet terug, en hoort dat alleen via het logbestand. Veiligheid wint hier van zichtbaarheid, maar niet gratis — het verlies is wél omkeerbaar (het bestand blijft onaangeroerd staan, dus het is te openen met ander gereedschap of te verkleinen), en de grens van 16 MiB komt bij echt tekenwerk neer op uren aaneengesloten annoteren. Het zichtbaar maken vraagt een eigen waarschuwingskanaal plus 31 vertalingen — het bestaande kanaal is `ChartDataWarning` en zou de verkeerde zin tonen. Dat is eigen werk met een eigen afweging, geen aanhangsel van een beveiligingsgrens, en gaat als aparte issue de lijst op. ## Poorten `make check` groen (5622 tests, dekking 86,2%). `make check-secrets` schoon (gitleaks over 1602 commits, trufflehog over werkboom én historie). `make sast` schoon (semgrep, 3 Dart-regels over 669 bestanden, 0 bevindingen). DAST niet gedraaid: ZAP is een webapp-scanner en de webbundel is CanvasKit — er valt niets te spideren, en deze wijziging raakt het geserveerde oppervlak niet. De klasseratchet viel onderweg om (`FileService`, 2938 tegen plafond 2904). De twee sidecar-helpers raken geen enkel veld van die klasse en staan nu buiten de klasse; de winst is vastgezet op 2885 in plaats van het plafond te verhogen. Refs #516
De sidecars naast een deck (.ink.json, .user-notes.json, .miauw.json,
.seal.json) werden onbegrensd ingelezen, met de kopie die jsonDecode er
meteen bovenop legt. Ze reizen met het deck mee — uit een pakket, een repo
of iemands map — dus de herkomst is dezelfde als die van grafiekdata, die
al wél een grens had.

De schrijfkant is de scherpste helft. Wat niet is ingelezen, mag ook niet
vervangen of verwijderd worden: het deck draagt de inhoud dan niet in het
geheugen, en opslaan wist andermans strepen of notities. Dat is precies de
redenering die er voor "uit een nieuwere build" al stond, dus lopen beide
aanleidingen nu door één poort — wat vier keer hetzelfde logblok scheelt.

Refs #516
Per record was er al een grens; op het archief zelf niet. De download schreef
door zolang de andere kant bleef sturen — Content-Length ging alleen naar de
voortgangsbalk — en het uitpakken van de geneste zip inflateerde net zo lang
als de stroom duurde. Een vervangen of omgeleide asset kon zo de schijf
volschrijven bij iemand die alleen op "binnenhalen" had gedrukt.

Beide helften toetsen nu de aankondiging én de werkelijk verwerkte bytes: de
aankondiging is gratis, de tweede is wat een liegende header opvangt. De
grens van de download staat apart in `streamCapped`, want `download` pint de
socket vast via NetGuard en die weigert loopback — een testserver op deze
machine is per ontwerp onbereikbaar, en zonder die scheiding was de grens
alleen te bewijzen door hem niet te toetsen.

Refs #516
De guard scande alleen uitgangen die deze app zélf opent. Een subproces en
een WebView openen hun eigen sockets — git doet zijn eigen DNS, de
webview-engine ook — dus safeResolve en socket-pinning bestaan daar niet, en
ze stonden daarom in het geheel niet in dit bestand.

Bij disk_traces.dart stond de reden al opgeschreven: de semgrep-uitzondering
zit daar bewust op de régel, "een tweede subproces hier moet wél alarm
geven". Alleen draait semgrep niet in `make check`, dus dat alarm bestond
nergens.

Wat er niet statisch te bewijzen valt is dát de beperking klopt; wat wél te
bewijzen valt is dat er geen uitgang bij komt zonder dat iemand het merkt.
Vandaar dezelfde vorm als voor HttpClient: allowlist per bestand plus een
telling, zodat ook een tweede uitgang in een bestaand bestand opvalt.

Refs #516
Een deny-list is nooit af. Deze had er drie gaten in gehad — <style>, SMIL's
<set attributeName="on…"> en puntkomma-lijsten — en elk daarvan was een
mechanisme dat naast de lijst liep in plaats van erin.

Het bezwaar dat in dit bestand stond was echt: een allow-list op gevoel laat
de eerste de beste sequentiediagram half renderen zónder foutmelding, en stil
verlies is erger dan een bekend gat. Maar de lijst hoeft niet geraden te
worden. flutter_svg is de enige lezer die deze SVG ooit krijgt, en de parser
erachter (vector_graphics_compiler) noemt zijn woordenschat expliciet:
_svgElementParsers, _svgPathFuncs, en de attributen die hij opzoekt. Alles
daarbuiten gooit die parser zélf al weg. De lijst is dus nooit strenger dan de
lezer — wat hier wegvalt, viel daar al weg — en dat is wat de ruil omdraait.

Geweigerd wordt gelogd, niet stil weggelaten: gaat Mermaid ooit iets anders
uitsturen, dan hoort dat in het log te staan en niet in een half diagram.

Dat het uitmaakt bleek meteen: x1/y1/x2/y2 ontbraken in de eerste versie van
de lijst, waardoor elke <line> een punt werd. De toets kijkt daarom naar de
attributen en niet alleen naar de elementnamen.

Refs #516
fix(svg): log het aantal weggevallen knopen, niet hun namen
Some checks failed
CI / Gate (Linux) · Format · Analyze · Coverage (push) Failing after 4s
CI / Web hardening (push) Failing after 4s
CI / Docs links (push) Failing after 4s
CI / Supply-chain (Trivy · advisory) (push) Failing after 6s
CI / Gate (Linux) · Format · Analyze · Coverage (pull_request) Failing after 4s
CI / Web hardening (pull_request) Failing after 4s
CI / Docs links (pull_request) Failing after 4s
CI / Supply-chain (Trivy · advisory) (pull_request) Failing after 5s
CI / Test (macos-latest) (push) Has been cancelled
CI / Test (windows-latest) (push) Has been cancelled
CI / Test (macos-latest) (pull_request) Has been cancelled
CI / Test (windows-latest) (pull_request) Has been cancelled
a877b6103f
De log_no_content-poort ving dit terecht: een elementnaam voelt als
structuur en niet als inhoud, maar hij komt uit een document dat de gebruiker
heeft geschreven, en een attribuutnaam is in een gemaakt diagram vrij te
kiezen. Het aantal zegt de ontwikkelaar wat hij nodig heeft — er viel iets
weg, ga kijken — en het logbestand hoeft geen tweede kopie van het diagram te
dragen.

Meteen ook de klasseratchet weer onder zijn plafond: de twee sidecar-helpers
raken geen enkel veld van FileService en staan nu buiten de klasse. De winst
is vastgezet in classSizeBaseline (2904 -> 2885).

Refs #516
brenno merged commit 42ec3d0811 into main 2026-07-22 14:28:37 +00:00
Sign in to join this conversation.
No description provided.