PHP: Come difendersi dal problema dell'SQL Injection

Tuesday, 19 December 06
Nel precedente post ho scritto del refactoring effettuato sul codice di oknotizie la scorsa settimana. Ancora il nuovo codice non e' online, ma una delle modifiche piu' interessanti che abbiamo operato (oltre alla separazione della implementazione del DB dal resto del programma, ora mediato da una API) e' la quella del potenziamento di mysql.php, libreria scritta da noi qualche tempo fa e utilizzata sia su Segnalo che Oknotizie.

Di norma PHP forza il programmatore a fare l'escape manuale dei parametri delle query SQL. Cio' e' chiaramente disumano. Ad esempio bisogna scrivere:
$name = mysql_escape_string($name);
$sql = "SELECT * FROM rubrica WHERE name='$name'";
$res = mysql_query($sql) ...
Nel nostro codice mysql_query non viene mai utilizzata direttamente perche' e' mediata da altre funzioni come mysqlQuery o mysqlQueryResult e altre simili che si occupano di gestire la connessione al DB se ancora questa non fosse attiva, il caching, ritornare direttamente la prima row, ed altri dettagli troppo tediosi da gestire a mano.

Sin da quando abbiamo iniziato a scrivere le due applicazioni, una cosa mi era abbastanza chiara, prima o poi un escape ce lo saremmo dimenticati. Tutti i linguaggi che utilizzo quando posso scegliere (evito il PHP come la peste) utilizzano una API abbastanza furba da fare l'escape da soli, la sicurezza non puo' essere affidata alla capacita' di non dimenticare mai un escape.

Bisognava porre rimedio, e a tale scopo ho scritto la seguente funzione, utilizzandola poi nel resto della libreria mysql.php:
function mysqlCreateQuery() {
    return mysqlCreateQueryVector(func_get_args());
}

function mysqlCreateQueryVector($argv) { # Build the query escaping where needed $argc = count($argv); $q = $query; for ($i = 0; $i < $argc; $i++) { if ($i&1) { if (is_array($argv[$i])) { $aux = ""; foreach($argv[$i] as $x) $aux .= "'".mysql_escape_string($x)."',"; $q .= trim($aux,","); } else { $q .= "'".mysql_escape_string($argv[$i])."' "; } } else { $q .= $argv[$i]; } } return $q; }
Le funzioni mysqlCreateQuery e mysqlCreateQueryVector sono praticamente identiche, la differenza e' che l'input viene passato ad una come una serie di argomenti e all'altra come un array contenente piu' elementi.

Cosa fa questa funzione? Costruisce query utilizzando l'escape automatico.

Ad esempio scrivere:
$sql = mysqlCreateQuery("SELECT * FROM rubrica WHERE name=",$name);
e' esattamente equivalente a scrivere:
$name = mysql_escape_string($name);
$sql = "SELECT * FROM rubrica WHERE name='$name'";
La funzione si occupa di fare l'escape del parametro e aggiungere i due apici all'inizio e alla fine.

Ovviamente le query non sono sempre cosi' semplici, cosi' la funzione e' abbastanza furba da trattare il primo argomento come una stringa normale, il secondo come una stringa con cui fare l'escape, il terzo ancora come normale, il quarto escape e cosi' via. Per cui e' possibile scrivere qualcosa come:
$sql = mysqlCreateQuery("SELECT id FROM news WHERE time>",$t,"AND user_id=",$uid);
Tuttavia ci sono casi in cui questa sintassi e' scomoda. Ad esempio durante una insert bisognerebbe scrivere:
$sql = mysqlCreateQuery("INSERT INTO foobar (a,b,c) VALUES (",$a,",",$b,",",$c,")");
E' il tripudio delle virgole e degli apici. Cosi' la funzione e' stata modificata in modo che se l'argomento da espandere e' un array invece di una stringa, viene espanso a escape($a[0]),escape($a[1]),escape($a[2]) e cosi' via. Dunque la precedente query puo' essere scritta semplicemente nella seguente forma:
$aux = Array($a,$b,$c);
$sql = mysqlCreateQuery("INSERT INTO foobar (a,b,c) VALUES (",$aux,")");
In mysql.php non usiamo queste funzioni in maniera diretta. Ce ne sono alcune di piu' alto livello come gia' detto che ci semplificano la vita. Ad esempio mysqlQueryResult crea la query con mysqlCreateQuery, collega il DB se e' sconnesso, esegue la query e ritorna il primo risultato (e lascia il DB connesso e selezionato per le prossime query).

Allo stesso modo mysqlQueryList si occupa di fare il foreach all'interno per ritornare un array di rows, e cosi' via.

Nel layer di livello superiore, dbapi.php, ci sono le funzioni che a loro volta utilizzano queste per dare maggiore comodita' al programmatore. Ad esempio la dbCount permette di scrivere una query come la seguente:
$row = mysqlQueryResult("SELECT COUNT(*) FROM news WHERE user_id=",$uid);
$count = $row[0];
in maniera piu' compatta come:
$count = dbCount("news","user_id=",$uid);
Anche la dbCount utilizza internamente mysqlCreateQuery.

E infatti...

Tanto e' vero che dovevamo preoccuparci che qualche giorno fa un utente ci ha segnalato che oknotizie era vulnerabile a SQL injection. Ci eravamo dimenticati un escape in un file PHP che si occupava di controllare via Ajax se la news era probabilmente duplicata. Era ovvio che sarebbe successo, e infatti e' accaduto :)

Non c'e' stato nessun problema reale e il baco e' stato corretto, ma mi dispiace che non siamo riusciti a mettere online la versione piu' sicura di oknotizie in tempo. Con la nuova interfaccia siamo piu' sicuri, e per tale motivo le semplici funzioni sopra esposte sono a disposizione di tutti sotto la licenza GPL: non ci vuole niente a riscriverle, ma e' un gesto di full disclosure come quello operato dal gentile utente che ci ha informati della vulnerabilita'.
28256 views*
Posted at 13:14:18 | permalink | 15 comments | print
Do you like this article?
Subscribe to the RSS feed of this blog or use the newsletter service in order to receive a notification every time there is something of new to read here.

Note: you'll not see this box again if you are a usual reader.

Comments

emix writes:
20 Dec 06, 04:38:33
Domanda: perché hai usato PHP per questi due progetti?
antirez writes:
20 Dec 06, 05:49:24
Ciao Emix,
Segnalo e Oknotizie sono stati fatti esplicitamente perche' pensavamo che sarebbero diventati oggetto di un accordo con qualche grosso operatore del settore. In tale ottica ci sembrava che la buzzword PHP potesse fungere da lascia passare, in maniera simile a Java, ma tra i due preferivamo di gran lunga PHP.

Non sapevamo ancora con chi avremmo fatto l'accordo e in quali termini, ma utilizzare un linguaggio non "mainstream" avrebbe potuto far temere al nostro partner un effetto lock-in perche' loro in caso di cessione dei sorgenti potrebbero non aver avuto le competenze interne per scrivere codice in tali linguaggi (in particolare Tcl, che era quello che desideravo usare io).

Per i prossimi progetti di Merzia ci sposteremo con buona probabilita' verso Ruby. E' un ordine di grandezza migliore di PHP e ormai e' abbastanza conosciuto e causa minore preoccupazione di Tcl :) In ogni caso per i nuovi progetti il problema e' meno severo perche' una partnership non e' piu' il nostro obiettivo principale, ma il supporto in termini di librerie e comunita' rimane importante dunque Ruby potrebbe essere il candidato ideale.

Difficilmente pero' useremo Rails, perche' nella nostra filosofia di sviluppo i framework sono considerati come portatori di problemi piu' che un vantaggio.
emix writes:
20 Dec 06, 06:36:57
Ottima scelta quella di Ruby! Lo uso già da un po' e da qualche settimana sto giochicchiando con Rails. Il framework è spettacolare, ma in effetti la sensazione che si ha è quella di essere un po' troppo abbottonati.
www.dario-avellino.it writes:
28 Dec 06, 08:26:08
Ciao ho fatto la tesina su questo argomento!
A presto.
Fra_T writes:
11 Apr 07, 12:09:14
Interessanti queste funzioni, solo che se anziché scrivere:

mysqlCreateQuery("SELECT * FROM rubrica WHERE name=",$name);

Scriviamo:

mysqlCreateQuery("SELECT * FROM rubrica WHERE name=$name");

$name salta mysql_escape_string. O sbaglio?
antirez writes:
11 Apr 07, 16:51:15
@Fra_T: esatto, bisogna usare la virgola. Non c'e' modo per evitare l'interpolazione se l'utente la usa esplicitamente, la cosa buona e' che pero' quando questo serve (ad esempio se una parte dell'SQL e' prodotto da una funzione diversa) e' possibile evitare il quoting automatico.

Ormai usiamo queste funzioni da alcuni mesi, l'utilizzo della virgola dopo un po' diventa cosi' automatico che e' molto difficile sbagliare.
Fra_T writes:
16 Apr 07, 11:22:22
Ho iniziato ad usare questa funzione e in effetti la trovo molto comoda :D
antirez writes:
16 Apr 07, 11:57:54
@Fra_T: cool :)
ThRiX writes:
17 May 07, 18:39:56
Ciao a tutti!
sto cercando una soluzione VALIDA! per evitare problemi (vedi sql injection appunto) sul mio gestionale.
Ho notato che nella descrizione della funzione, non viene descritto il suo utilizzo con il comando UPDATE. Ho provato ad adattarla in qualche modo senza esiti positivi!

Ciao a tutti...
<?php $query = "SELECT * FROM users WHERE user='".$_POST['user']."' AND pwd='".$_POST['pwd']."'"; $sql = mysql_query($query); if(mysql_affected_rows($sql)>0) { } ?> writes:
06 Aug 07, 14:00:44
' writes:
23 Jan 08, 05:40:59
'
ruto writes:
18 Sep 08, 02:50:10
ottima funzione...anche se poi a parere mio ti impone di scrivere le query in un certo modo...sarebbe bello scrivere una query normale senza stare attento a metter " etc...

cmq il io giudizio e' positivo..complimenti!
Marco Grazia writes:
17 Mar 09, 05:20:13
Antirez cosa ne pensi del modo di interfacciarsi ai database proposto dalla libreria PDO in PHP?
Navigatore Anonimo writes:
20 Apr 09, 05:57:29
"Di norma PHP forza il programmatore a fare l'escape manuale dei parametri delle query SQL. Cio' e' chiaramente disumano."
non è vero...

"la sicurezza non puo' essere affidata alla capacita' di non dimenticare mai un escape"
significa che l'approccio iniziale è già sbagliato...

abbastanza banale.. bastava dire che in assenza di magic_quotes bisogna escapare tutte le stringhe in inserimento. Ciò è anche obbligatorio per legge

adottare un framework serio ovvia tutti questi problemi.
magomerlino writes:
27 Mar 10, 03:42:00
Ciao a tutti,
per difendersi da sql injection non basta sanizzare l'entrata, purtroppo bisogna fare la validazione completa :-( Cetamente sanizzare l'input ti compre dal 70 o 80% degli attacchi ed attaccanti ma .... Como fare una validazione degli input ed output? Allora pima bisgna fare una caninicalizzazione (convertire l'input in una forma stadard che non possa essere alterata, es: utf-8), pi viene la sanizzazzione per i caratteri pericolosi che dobbiamo permettere come input (es: se en un messagio dobbiamo permettere ' e quindi lo sanizzaimo per esempio con funzioni come mysql_real_escape) ed infine viene la vera validazione, permettere solo cio che e permesso come entrata: controllare che l'input rispetta rango di caratteri o numeri, formato, dimensione (es: se e un numeor di telefono si controlla la lunghezza, che contenga solo numeri [0-9] e che abbia il formato corretto); questo si puo fare con espressioni regolari. Se inoltre volete fare dei test de intrusione potete scegliere diverse soluzioni presenti nel mercato, dalle free (sql me,...) alle commerciali. Vi consiglio guardare www.security-guardian.com sono dei fuori classe. attualemnte il servizio e gratis. Fanno dei test online della web e nell'area di amministrazione ti mostrano i risultati con le soluzioni che bisogna implementare per risolvere le vulnerabilita detettate.
Saluti
comments closed