02Sicurezza5 min

Quello che ho imparato auditando il mio codice come se fosse di un altro

Un ACL vuoto che restituiva l'intero corpus, un'iniezione di CSS senza una riga di JavaScript e una API key che valeva ancora dopo aver disattivato il proprietario. Tutto in un giorno, tutto in codice che avevo scritto io.

Di Salva Castells

Immagine di copertina: Ildar Sagdejev (Specious), CC BY-SA 3.0.
In questo post

In un solo giorno ho chiuso sei falle di sicurezza in una API di trascrizione che girava da mesi. Nessuna l'ha trovata un test. I test passavano in verde prima e dopo. Quello che è cambiato non è stato lo strumento: è stato che mi sono messo a leggere il mio codice chiedendomi come lo romperei se volessi farlo, invece di chiedermi se faceva quello che avevo pensato.

Quella differenza sembra di atteggiamento. È di metodo, e si vede nel tipo di falla che salta fuori.

Un test verifica il percorso che hai immaginato

Un test lo scrivi partendo dal caso d'uso: dati ragionevoli, output ragionevole. Un attaccante non passa dati ragionevoli. Passa il vuoto, lo zero, il negativo, l'enorme. Quei valori stanno raramente nella suite, perché quando scrivi il test non ti vengono in mente e quando ti vengono in mente hai già visto il bug.

Il caso più chiaro è stato un ACL vuoto. Il motore di recupero riceveva una lista di identificatori a cui l'utente aveva accesso e filtrava il corpus con quella. Se la lista arrivava vuota, il codice la interpretava come "non c'è nessun filtro da applicare" invece che come "questo utente non ha accesso a niente". Restituiva l'intero corpus.

Quello falliva in apertura: il valore di default di un'assenza di permessi era permettere tutto. Nessun test del percorso felice lo tocca mai, perché nel percorso felice la lista porta sempre qualcosa.

Da allora rivedo tutti i filtri di autorizzazione con la stessa domanda: cosa fa questo se la collezione è vuota? Non "cosa fa se l'utente non ha permessi", che di solito è ben coperto, ma cosa fa con il vuoto.

Le categorie si ripetono più di quanto sembri

I risultati si raggruppano quasi sempre nelle stesse famiglie.

  • Permessi che non si richiedono dove serve. Un endpoint di esportazione che lasciava tirare fuori dati senza chiedere l'autorità di gestione della struttura. Una cancellazione a cascata di un dominio che controllava il permesso una volta all'inizio invece di controllare l'autorità di cancellazione dura su ogni elemento coinvolto. La cascata è il posto classico: autorizzi l'operazione e poi il ciclo interno fa cose che nessuno ha autorizzato.
  • Limiti che non esistono. Gli elenchi di amministrazione passavano il limit che arrivava diretto alla query, senza tetto. Un valore grande da uno script materializzava l'intera tabella nella memoria del worker. La correzione sta in una riga, Math.min(Math.max(limit, 1), 1000), ma non c'era perché nella UI il limit valeva sempre dieci.
  • Stato che si controlla una volta e mai più. authenticateApiKey validava l'hash, la scadenza e il flag isActive della chiave. Ma la riga della chiave sopravvive allo stato dell'account del proprietario: se il record di fatturazione viene ripulito quando disattivi qualcuno, nessuno abbassa quel flag e la chiave continua ad autenticare. Ora la funzione rivalida che il proprietario abbia ancora un record di fatturazione, in un posto solo, così che tutti gli endpoint che autenticano per chiave lo ereditino invece di doverselo ricordare ciascuno.
  • Contenuto di terzi che viene renderizzato. Qui c'è quello che mi ha sorpreso di più.

Anche il CSS è un'iniezione

Il sanificatore di markdown permetteva gli attributi style e class liberi su div, span e svg. La catena va da raw a sanitize a harden e nessuna fase guarda dentro il CSS: la libreria di sanificazione tratta style come stringa opaca e l'hardener riscrive solo le URL in a e img. Lo stile arrivava intatto al render finale.

Ho pensato di scrivere un parser di CSS con lista bianca di proprietà. Non l'ho fatto: ho verificato che nessuna vista legittima avesse bisogno di style e ho tolto i due attributi dalla lista permessa. Un filtro di CSS fatto in casa è superficie nuova da mantenere per coprire un caso che non esisteva.

L'importante della scoperta non è la correzione, è la classe di vettore. Non serve JavaScript per attaccare un altro utente: con il CSS puoi coprire un pulsante, spostare un testo sopra un altro o cambiare quello che sembra dire un messaggio. E qualsiasi vista che disegni contenuto altrui, non solo il proprio, è un vettore cross-user.

Due scoperte che non sono bug di logica, ma di ciclo di vita.

La prima: quando l'autenticazione per API key falliva per chiave corrotta o malformata, il catch metteva la chiave in chiaro nel contesto del log, e il logger serializzava quel contesto con JSON.stringify senza redigere niente. Una chiave valida mandata a un endpoint sbagliato finiva in testo chiaro nel log. La redazione che ho messo attacca da due lati, per nome di campo (/rawkey|token|authorization|password/i) e per forma del valore (il prefisso dk_live_ / dk_test_), e lascia visibili i primi otto caratteri seguiti da ***REDACTED***, lo stesso formato con cui la UI rinasconde la chiave dopo averla mostrata una volta.

La seconda: le URL firmate di S3 per il player audio si generavano con expiresIn di sette giorni. Sette giorni sono un'eternità per un link che deve solo sopravvivere al caricamento di un <audio>. Chiunque avesse il link — inoltrato, nella cronologia, messo in cache da un proxy aziendale — conservava l'accesso all'audio originale per tutta quella finestra, e cancellare la trascrizione non revocava niente. Ho abbassato la scadenza a 600 secondi, con rinnovo dal client a 480, e ho aggiunto il DeleteObjectCommand sull'oggetto quando si cancella la trascrizione, perché fino ad allora la cascata cancellava solo la riga.

Il costo in esperienza utente è stato zero tranne per un flusso: inoltrare il link dell'audio via email a un collega. Quel flusso non è mai stato una funzionalità supportata né aveva alcun controllo di accesso, e mi sembra corretto che abbia smesso di funzionare.

Ora lo pianifico invece di ricordarmelo

Il problema degli audit è che dipendono dal fatto che un giorno ti vada. Il mio è uscito perché stavo documentando decisioni di design e rileggendo mi sono stonate delle cose.

Ogni scoperta diventa un file breve in docs/security/ con il vettore e il perché la correzione è quella e non un'altra, e da lì esce la lista con cui rivedo il resto. Quando ho ripetuto l'esercizio sul boilerplate con cui avvio i progetti sono usciti trentanove punti tra sicurezza, SEO e architettura, e la maggior parte erano versioni dello stesso pugno di domande: cosa succede con la lista vuota, chi richiede il permesso nel ciclo interno, qual è il tetto di questo limite, quanto vive questo link.

Leggere il tuo codice per confermare che funziona e leggerlo per vedere come romperlo sono due letture diverse. La seconda non avviene da sola mentre fai la prima.