Securitycodereview uitgelegd: waar een reviewer in een diff naar kijkt, en waarom

Een beveiligingsreview van code is het bewust lezen van broncode met als specifiek doel manieren te vinden waarop de code iets kan laten doen wat de auteur niet bedoelde. Die definitie klinkt vanzelfsprekend tot je haar vergelijkt met wat de meeste teams in werkelijkheid doen: een functionele review met een beveiligingschecklist in de buurt. De OWASP Secure Code Review Cheat Sheet trekt de grens precies: een beveiligingsreview verschilt van een functionele review doordat deze zich richt op invoervalidatie, authenticatie, autorisatie, cryptografische implementatie en aanvalsvectoren, in plaats van op correctheid. Een diff die perfect werkt, kan een aanvaller nog steeds een toegangspad naar het systeem bieden, en slechts één van die twee reviews is ernaar op zoek.

Wat dit onderwerp nu een artikel waard maakt in plaats van in 2020, is dat een taalmodel een diff bekwaam, goedkoop en continu kan lezen — wat de economie van beoordeling verandert zonder ook maar één van de dimensies die worden beoordeeld te veranderen. De checklist is ouder dan de modellen en is niet veranderd. Wat is veranderd, is hoe vaak je het je kunt veroorloven om hem uit te voeren.

Waar een recensent eigenlijk naar op zoek is tijdens het lezen

De OWASP cheat sheet organiseert het werk in zes gebieden, en het nuttige van de lijst is dat elk gebied een vraag is in plaats van een categorie. Categorieën nodigen uit tot afvinken; vragen leveren bevindingen op.

Invoer en injectie. De vraag is niet of de code een sanitizer heeft; de vraag is of er iets een interpreter bereikt — shell, SQL, template-engine, expressie-evaluator — en op welke laag die vereiste gehandhaafd hoort te worden. Een diff die een geparametriseerde query toevoegt is goed; een diff die een geparametriseerde query toevoegt aan een functie waarvan de andere aanroepers ruwe strings doorgeven, is een diff die het probleem heeft verplaatst.

Authenticatie en sessie. Wie is de beller, en hoe heeft de code dat vastgesteld? De terugkerende bug is niet een kapotte wachtwoordcontrole, maar een sessie die een bevoegdheidswijziging overleeft — er vindt elevatie plaats, het token behoudt zijn oude claims, en een uur later zijn de privileges er nog steeds.

Autorisatie.Controles op objectniveau, elke keer, op elke route. Waar je in een diff specifiek op moet letten, is een nieuwe route die de guard van de andere routes onder dezelfde router niet erft, omdat middleware per router wordt geregistreerd en een route die onder een andere router wordt toegevoegd simpelweg niet wordt gedekt.

Cryptografie. Primitief, modus, sleutellengte en waar de sleutel zich bevindt. Twee vragen vangen de meeste echte bevindingen: kan iedereen die deze codebase kan lezen ook de sleutel lezen, en is deze vergelijking er een die geen timing mag lekken.

Bedrijfslogica. Of de volgorde kan worden herschikt, opnieuw afgespeeld of aangestuurd door waarden die de client heeft gekozen. Een prijs, een hoeveelheid, een rol of een statusovergang die van de client komt en niet opnieuw aan de serverzijde wordt afgeleid, is een bevinding, ongeacht hoe deze wordt gevalideerd.

Configuratie en secrets. Wat de diff meelevert dat nooit in een repository had mogen staan, en of de standaardwaarden veilig zijn op de plek waar de code daadwerkelijk draait. Een debug-flag die standaard op true staat is een bevinding over productie, niet over de flag.

Baseline-review versus diff-review

OWASP onderscheidt twee modi, en het door elkaar halen daarvan is een veelvoorkomende bron van zowel verspilde inspanning als valse zekerheid. Een basisreview onderzoekt de codebase uitgebreid, en het is het juiste hulpmiddel voor een nieuwe applicatie of een grote release, het onboarden van een legacy-systeem, het voldoen aan een compliancevereiste, of het uitvoeren van de post-incidentanalyse nadat er al iets mis is gegaan. Een diff-gebaseerde review kijkt alleen naar wat er is veranderd, en is het juiste hulpmiddel voor pull requests, commits, het afronden van features en continue validatie.

Het onderscheid is belangrijk omdat de twee verschillend werk opleveren. Een baseline-review eindigt met een backlog; een diff-review eindigt met een mergebeslissing. Het eerste continu laten draaien is onbetaalbaar, en het tweede draaien op een codebase waarop het eerste nooit is toegepast, is hoe teams een vals gevoel van dekking opbouwen: elke wijziging komt door de review, en de bestaande SQL-concatenatie blijft onaangeroerd liggen.

Technieken, en wat elk ervan opspoort

OWASP noemt vier reviewtechnieken, en ze onderscheiden zich door welk bewijs de reviewer volgt, niet door hoe grondig ze zijn. Codepatroonanalyse volgt bekende patronen: de gevaarlijke functieaanroep, het deserialiseren van niet-vertrouwde invoer, de ontbrekende lengtecontrole. Het is snel en het is precies wat een deterministisch hulpmiddel goed doet. Dataflow-analyse volgt een waarde vanaf waar deze binnenkomt tot waar deze wordt gebruikt, en dat is de enige manier om te beantwoorden of de sanitizer die drie functies geleden is toegepast hier nog steeds geldt. Dreigingsgerichte review begint bij de capaciteit van een aanvaller en werkt vooruit, en zo vind je de kloof tussen twee afzonderlijk correcte componenten. Bedrijfslogica-review leest de workflow in plaats van de regels, en vraagt welke reeks legitieme bewerkingen een illegitieme toestand oplevert.

Alleen de eerste van die vier wordt goed bediend door een scanner, en alleen de eerste en tweede worden goed bediend door een ongestuurde modeldoorloop. Dat is geen argument tegen modellen; het is een argument om de reviewer te vertellen welke techniek je wilt, omdat een prompt die om "security issues" vraagt betrouwbaar patroonanalyse oplevert en dan stopt.

Wat AI veranderde, en wat niet

Onderzoek naar door AI geschreven code maakte het reviewknelpunt scherper in plaats van het te creëren. De NYU-studie naar GitHub Copilot-suggesties, gepubliceerd bij IEEE S&P in 2022, vond dat ongeveer 40% van de gegenereerde snippets een exploiteerbaar patroon bevatte across 18 CWE-categorieën. Het Stanford-onderzoek naar ontwikkelaarsgedrag, gepubliceerd bij ACM CCS in 2023, vond het ingrijpender effect: ontwikkelaars die assistenten gebruikten, waren er zekerder van dat hun code veilig was, terwijl ze aantoonbaar minder veilige code produceerden. Meer output plus meer vertrouwen is een reviewprobleem.

Een taalmodel als reviewer verandert één variabele in die vergelijking: doorvoer. Het kan elke pull request lezen, tegen kosten per token in plaats van per seat, en men kan het expliciet de zes bovenstaande vragen stellen. Het verandert de vragen niet, en het verwerft niet de twee eigenschappen die een review betrouwbaar maken: determinisme en het vermogen om de code uit te voeren. Dezelfde diff die twee keer wordt gereviewd, kan verschillende bevindingen opleveren, ernstniveaus verschuiven tussen runs, en de reviewer kan niet bewijzen dat een gemarkeerd pad bereikbaar is.

OrcaCode Review is de productvorm van dat idee, en het is de moeite waard er nauwkeurig naar te kijken, omdat de ontwerpkeuzes de faalmodi onthullen. Een review die het falende geval isoleert als een specifieke, reproduceerbare opmerking — deze sleutel varieert nog in de tijd, dus een opnieuw geprobeerde afschrijving genereert een nieuwe sleutel en kan dubbel afschrijven, hier is de wijziging van één regel — doet het werk dat een reviewer doet. Een review die "potentieel beveiligingsprobleem" meldt zonder reproductie, doet aan patroonanalyse met slechtere precisie dan de patroonmatcher. En een regels- en poortengine die onder een open licentie is gepubliceerd, doet een impliciete claim die het checken waard is: dat ook het oordeel van de reviewer reviewbaar moet zijn.

De tweelaagse opstelling die werkt

Gegeven een deterministische scanner en een probabilistische reviewer, is de opzet die in de praktijk standhoudt om ieder het werk te geven waar hij daadwerkelijk goed in is. De scanner draait bij elke push: seconden, geen model, geen variantie, en vangt secrets, gevaarlijke API-aanroepen, onveilige deserialisatie en gehallucineerde imports. De reviewer draait bij elke pull request: minuten, modelondersteund, en vangt de logica tussen de regels — of de autorisatiecontrole vóór de fetch plaatsvindt, of de idempotentiesleutel aan een stabiele waarde is gebonden, of de nieuwe route de guard heeft geërfd.

Dan de laag die mensen overslaan: de poort. Een bevinding die een merge niet blokkeert, is een melding, en meldingen worden op vrijdag gelezen. De poort moet smal en absoluut zijn — kritieke bevindingen blokkeren, al het overige levert commentaar op — en de vrijstelling moet op papier staan, want de waarde van een poort is niet dat hij de eerste bevinding tegenhoudt, maar dat hij elke omzeiling zichtbaar maakt.

De wijziging beoordelen, niet de codebase

De praktische discipline voor een diff-review is een korte reeks, en de waarde ervan is dat je die in een middag kunt aanleren. Lees de diff voor wat hij toevoegt aan het aanvalsoppervlak, niet voor wat hij verandert. Volg bij elk nieuw toegangspunt de invoer één stap voorbij waar de diff eindigt — de kwetsbaarheid zit meestal in de functie die hij aanroept, die de auteur niet heeft gelezen. Controleer bij elke nieuwe route de registratie van de guard. Controleer bij elke nieuwe dependency of die bestaat en of iemand anders de naam heeft gepubliceerd. Vraag je bij elke nieuwe configuratiewaarde af wat de standaardwaarde is op de plek waar die daadwerkelijk wordt uitgerold.

Niets daarvan vereist een goede kennis van de taal, en daarom schaalt het naar reviewers die geen securityspecialisten zijn. Het vereist wel dat de reviewer de diff ziet in plaats van het bestand, omdat de hele methode afhangt van weten wat nieuw is.

En de eerlijke beperking: een diffreview kan de fout die er al vóór de diff was, niet vinden. Als de applicatie nooit een basisreview heeft gehad, is het waardevolste beveiligingswerk dat beschikbaar is niet een betere reviewer — het is één bewuste doorgang over de codebase, in de gebieden die geld, identiteit en bestandstoegang raken, met de zes bovenstaande vragen. Daarna beschermt elke volgende diffreview iets dat één keer is bekeken. Daarvoor beschermt elke diffreview iets onbekends.