fix: zet vijf punten uit de beveiligingsronde recht (#516) #560
No reviewers
Labels
No labels
accepted
bug
declined
docs
duplicate
enhancement
good first issue
in-progress
needs-info
privacy
security
triage
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
LibreKAT/Ocideck!560
Loading…
Reference in a new issue
No description provided.
Delete branch "sec/sidecar-leesgrens"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
jsonDecodeerbovenop 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_testvoorProcess.startenWebViewController. Bijallebei opent iets anders dan deze app de socket —
gitdoet zijn eigen DNS, dewebview-engine ook — dus
safeResolveen socket-pinning bestaan daar niet, enze stonden daarom in het geheel niet in dit bestand. Bij
disk_traces.dartstondde 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_svgvan deny-list naar allow-list. Het bezwaar dat in dat bestandstond 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_svgis deenige lezer die deze SVG ooit krijgt, en de parser erachter
(
vector_graphics_compiler) noemt zijn woordenschat expliciet. Alles daarbuitengooit 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/y2ontbraken in de eerste versie vande lijst, waardoor elke
<line>een punt werd. De toets kijkt daarom naar deattributen 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 rustlaat.
cve_bulk_ingest_test.dart— een te grote aankondiging, en een zip die overzijn 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:downloadpint desocket 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 beiderichtingen.
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
ChartDataWarningen zou de verkeerde zin tonen. Dat iseigen werk met een eigen afweging, geen aanhangsel van een beveiligingsgrens, en
gaat als aparte issue de lijst op.
Poorten
make checkgroen (5622 tests, dekking 86,2%).make check-secretsschoon(gitleaks over 1602 commits, trufflehog over werkboom én historie).
make sastschoon (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). Detwee 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