Scheda del titolo per 'Security Code Review Explained' che mostra un pannello diff del codice con una riga originale e una modificata, e tre fasi etichettate sotto: PATTERNS (deterministico), poi PASS, poi REASONING (modello).
Guides & Insights

Revisione del codice di sicurezza spiegata: cosa cerca un revisore in un diff, e perché

Autore

Rowan Sterling

Data di pubblicazione

Ultimi modelli · 20Vedi tutti i modelli →
Benchmark: Artificial Analysis · aggiornato ogni giorno
Torna a tutti gli articoli

Una revisione di sicurezza del codice è la lettura intenzionale del codice sorgente al preciso scopo di individuare i modi in cui lo si può indurre a fare qualcosa che il suo autore non intendeva. Questa definizione sembra ovvia finché non la si confronta con ciò che la maggior parte dei team esegue effettivamente, ovvero una revisione funzionale con una checklist di sicurezza a portata di mano. L'OWASP Secure Code Review Cheat Sheet traccia la distinzione con precisione: una revisione di sicurezza si differenzia da una revisione funzionale perché si concentra su validazione degli input, autenticazione, autorizzazione, implementazione crittografica e vettori di attacco, anziché sulla correttezza. Una diff che funziona perfettamente può comunque offrire a un attaccante una via d'accesso al sistema, e solo una di queste due revisioni la sta cercando.

Ciò che rende l'argomento meritevole di un articolo ora anziché nel 2020 è che un modello linguistico può leggere un diff in modo competente, a basso costo e continuamente — il che cambia l'economia della revisione senza cambiare una sola delle dimensioni oggetto di revisione. La checklist è più vecchia dei modelli e non è cambiata. Ciò che è cambiato è quanto spesso puoi permetterti di eseguirla.

Che cosa sta effettivamente cercando un recensore

Il cheat sheet di OWASP organizza il lavoro in sei aree, e la cosa utile di questo elenco è che ogni area è una domanda anziché una categoria. Le categorie invitano a spuntare caselle; le domande producono riscontri.

Card 'What a reviewer is actually looking for' listing six dimensions - input and injection, authn and session, authorization, cryptography, business logic, configuration and secrets - each with the question it answers about a diff.

Input e injection. La domanda non è se il codice disponga di un sanitizzatore; è se qualcosa raggiunga un interprete — shell, SQL, motore di template, valutatore di espressioni — e a quale livello quel requisito debba essere applicato. Una diff che aggiunge una query parametrizzata è una buona diff; una diff che aggiunge una query parametrizzata in una funzione i cui altri chiamanti passano stringhe grezze è una diff che ha spostato il problema.

Autenticazione e sessione. Chi è il chiamante, e come ha fatto il codice a stabilirlo? Il bug ricorrente non è un controllo della password difettoso, ma una sessione che sopravvive a un cambio di privilegi: l'elevazione avviene, il token mantiene le sue vecchie attestazioni e, un'ora dopo, i privilegi sono ancora lì.

Autorizzazione. Controlli a livello di oggetto, ogni volta, su ogni rotta. La cosa specifica da cercare in un diff è una nuova rotta che non eredita la guardia che hanno le rotte sorelle, perché il middleware viene registrato per router e una rotta aggiunta sotto un router diverso semplicemente non sarà coperta.

Crittografia. Primitiva, modalità, lunghezza della chiave e dove risiede la chiave. Due domande individuano la maggior parte dei problemi reali: chiunque possa leggere questo codice può anche leggere la chiave? E questa comparazione è una di quelle che non devono rivelare informazioni temporali?

Logica di business. Se la sequenza può essere riordinata, riprodotta o guidata da valori scelti dal client. Un prezzo, una quantità, un ruolo o una transizione di stato che arriva dal client e non viene ricalcolato lato server costituisce un riscontro, indipendentemente da come venga validato.

Configurazione e segreti. Cosa introduce il diff che non sarebbe mai dovuto finire in un repository, e se i valori predefiniti sono sicuri là dove il codice viene effettivamente eseguito. Un flag di debug impostato su true per impostazione predefinita è una segnalazione che riguarda la produzione, non il flag.

Revisione baseline rispetto alla revisione diff

OWASP separa due modalità, e confonderle è una fonte comune sia di sforzi sprecati sia di falsa sicurezza. Una revisione di base esamina il codebase in modo completo ed è lo strumento giusto per una nuova applicazione o un rilascio importante, per l'onboarding di un sistema legacy, per soddisfare un requisito di conformità o per l'analisi post-incidente dopo che qualcosa è già andato storto. Una revisione basata sul diff guarda solo ciò che è cambiato ed è lo strumento giusto per pull request, commit, completamento di funzionalità e validazione continua.

La distinzione è importante perché le due producono un lavoro diverso. Una revisione di base si conclude con un backlog; una revisione del diff si conclude con una decisione di merge. Eseguire la prima in modo continuo è insostenibile, ed eseguire la seconda su una codebase che non ha mai avuto la prima è il modo in cui i team accumulano una falsa sensazione di copertura: ogni modifica supera la revisione, e la concatenazione SQL preesistente resta lì, intoccata.

Screenshot of the OWASP Cheat Sheet Series index entry for Secure Code Review, showing the introduction describing manual examination of source code for vulnerabilities automated tools miss, the baseline-versus-diff review split with their use cases, and a table of contents covering methodology, patterns, techniques, checklists and tools.

Tecniche, e cosa ciascuna rileva

OWASP elenca quattro tecniche di revisione, e si distinguono per il tipo di evidenza che il revisore segue piuttosto che per quanto siano approfondite. Analisi dei pattern di codicesegue forme familiari: la chiamata a funzione pericolosa, la deserializzazione di input non attendibile, il controllo della lunghezza mancante. È veloce ed è esattamente ciò che uno strumento deterministico fa bene. Analisi del flusso di datisegue un valore da dove entra a dove viene utilizzato, che è l'unico modo per rispondere se il sanitizzatore applicato tre funzioni fa sia ancora efficace qui. Revisione basata sulle minacceparte dalle capacità di un attaccante e procede in avanti, ed è così che si individua il divario tra due componenti singolarmente corrette. Revisione della logica di businesslegge il flusso di lavoro anziché le righe, chiedendosi quale sequenza di operazioni legittime produca uno stato illegittimo.

Solo il primo di quei quattro è ben servito da uno scanner, e solo il primo e il secondo sono ben serviti da una passata non guidata di un modello. Non è un argomento contro i modelli; è un argomento a favore del dire al revisore quale tecnica si desidera, perché un prompt che chiede "problemi di sicurezza" produrrà in modo affidabile un'analisi dei pattern e si fermerà.

Che cosa ha cambiato l’IA e che cosa no

La ricerca sul codice scritto dall'IA ha acuito il collo di bottiglia della revisione anziché crearlo. Lo studio della NYU sui suggerimenti di GitHub Copilot, pubblicato a IEEE S&P nel 2022, ha rilevato che circa il 40% degli snippet generati conteneva un pattern sfruttabile in 18 categorie CWE. Il lavoro di Stanford sul comportamento degli sviluppatori, pubblicato ad ACM CCS nel 2023, ha rilevato l'effetto più rilevante: gli sviluppatori che usavano gli assistenti erano più fiduciosi che il proprio codice fosse sicuro mentre producevano codice misurabilmente meno sicuro. Più output e più fiducia costituiscono un problema di revisione.

Un modello linguistico come revisore cambia una variabile in quell'equazione: il throughput. Può leggere ogni pull request, a un costo per token anziché per utente, e gli si possono porre esplicitamente le sei domande precedenti. Non cambia le domande e non acquisisce le due proprietà che rendono affidabile una revisione: il determinismo e la capacità di eseguire il codice. Lo stesso diff revisionato due volte può produrre risultati diversi, i livelli di gravità cambiano tra un'esecuzione e l'altra, e il revisore non può dimostrare che un percorso segnalato sia raggiungibile.

Screenshot of the OrcaCode Review product page on OrcaRouter headed 'Open engine. Open prompt. Open bill.', describing a pull-request reviewer whose rules and merge gate run on GitHub under an MIT licence and are billed per token, with an example finding about an idempotency key that still varies in time.

OrcaCode Review è la forma prodottizzata di quell’idea, e vale la pena esaminarla da vicino perché le scelte progettuali rivelano le modalità di fallimento. Una revisione che isola il caso di errore come un commento specifico e riproducibile — questa chiave varia ancora nel tempo, quindi un addebito ritentato genera una nuova chiave e può addebitare due volte, ecco la modifica da una riga — svolge il lavoro che fa un revisore. Una revisione che segnala "potenziale problema di sicurezza" senza una riproduzione sta facendo analisi di pattern con una precisione peggiore di quella del pattern matcher. E un motore di regole e gate pubblicato con licenza aperta avanza un’affermazione implicita che vale la pena verificare: che anche il giudizio del revisore dovrebbe essere sottoponibile a revisione.

La disposizione a due strati che funziona

Dato uno scanner deterministico e un revisore probabilistico, l'assetto che regge alla prova dei fatti è assegnare a ciascuno il lavoro in cui è davvero bravo. Lo scanner viene eseguito a ogni push: pochi secondi, nessun modello, nessuna variabilità, intercetta segreti, chiamate API pericolose, deserializzazione non sicura e import allucinati. Il revisore viene eseguito a ogni pull request: qualche minuto, supportato da un modello, intercetta la logica tra le righe — se il controllo di autorizzazione viene eseguito prima del fetch, se la chiave di idempotenza è legata a un valore stabile, se la nuova route ha ereditato la protezione.

Poi lo strato che la gente salta: il gate. Un finding che non blocca un merge è una notifica, e le notifiche si leggono il venerdì. Il gate deve essere ristretto e deve essere assoluto — i finding critici bloccano, tutto il resto si limita a commentare — e la deroga deve essere messa per iscritto, perché il valore di un gate non sta nel fermare il primo finding, ma nel rendere visibile ogni bypass.

Rivedendo la modifica, non il codebase

La disciplina pratica per una revisione del diff è una breve sequenza, e il suo valore è che può essere insegnata in un pomeriggio. Leggi il diff per ciò che aggiunge alla superficie d'attacco anziché per ciò che cambia. Per ogni nuovo punto d'ingresso, segui l'input per un passaggio oltre il punto in cui termina il diff — la vulnerabilità è di solito nella funzione da esso chiamata, che l'autore non ha letto. Per ogni nuova rotta, controlla la registrazione della guardia. Per ogni nuova dipendenza, controlla se esiste e se qualcun altro ha pubblicato il nome. Per ogni nuovo valore di configurazione, chiediti qual è il suo valore predefinito nel contesto in cui verrà effettivamente distribuito.

Niente di tutto ciò richiede una conoscenza approfondita del linguaggio, ed è per questo che il metodo si estende a revisori che non sono specialisti di sicurezza. Richiede invece che il revisore veda il diff anziché il file, perché l'intero metodo dipende dal sapere che cosa è nuovo.

E il limite onesto: una revisione del diff non può trovare il difetto che era già presente prima del diff. Se l'applicazione non ha mai avuto una revisione di base, il lavoro di sicurezza di maggior valore disponibile non è un revisore migliore — è una singola passata deliberata sul codice, nelle aree che riguardano denaro, identità e accesso ai file, con le sei domande sopra. Dopo, ogni revisione del diff successiva protegge qualcosa che è stato esaminato una volta. Prima, ogni revisione del diff protegge un'incognita.