[Pillola]Worst Practices - Abitudini da Evitare quando si programma

[kentaromiura]

Regola numero 1: Input is Evil


Scrivo questa pillola, sperando che chiunque apporti un contributo significativo,
lo scopo sarà elencare le peggiori abitudini dei programmatori (PHP ma non solo)
elencando per ogni ERRORE CLASSICO o infimo la via corretta, cercando di spiegare
di volta in volta il perchè non si debba agire nel modo sconsigliato;

1)Register_Globals

Partiamo col considerare una peculiarità di PHP , ovvero register_globals
questa opzione del file php.ini DEVE ESSERE SETTATA SEMPRE A OFF,

questa opzione permette a php di prendere ogni variabile passata tramite get o post
e trattarla come una variabile globale,
un opzione che potrebbe sembrar utile, ma che ha un piccolo svantaggio
chiunque puo' alterare qualsiasi variabile ,anche solo aggiungendo un parametro al get

se il registro fosse a ON anche un possibile "codice sicuro" come il seguente potrebbe venir bucato:

page.php

if($_POST['nick'])
    
    if(getPassWordFromDB(
               $_POST['nick'],
               $_POST['pass'])
         )
        $auth=1;
    else
        $auth=0;
    
if($auth==1)
        echo('UTENTE IDENTIFICATO'); 

la funzione getPassWordFromDB sarà una funzione che valida e cripta l'input del nick e della pass e restituira true
se nel db i dati corrispondono (ipotizziamo che questa funzione sia sicura al 100%)

in caso di register_global = off lo script sopra non causa problemi..
ma se settato a ON chiunque immettendo page.php?auth=1 vedrebbe la scritta UTENTE IDENTIFICATO.
(per provarlo basta sostituire if(getPassWordFromDB()) con if(false))

Quindi andate nel php.ini e impostate
register_globals = Off
e riscrivete i vostri script che si basavano su questo sistema utilizzando opportunamente
$_GET $_POST $_REQUEST $_COOKIE e $_SESSION

2)Validazione lato client & Code Injection
in molti effettuano dei controlli tramite JS per validare un input
e assicurarsi che l'input non sia potenzialmente dannoso,
ebbene le funzioni Js sono buone per evitare che per un errore di digitazione
si debba ricaricare una pagina,
bisogna sempre pero' controllare la validità dell' input lato Server, poiche'
chiunque potrebbe semplicemente disabilitando Js nel proprio Browser immettere qualsiasi codice dannoso
(per evitare il code injection si consiglia di effettuare sempre un str_replace("<","&lt;",$input);

3)Variabili miste a SQL
una brutta abitudine abitudine e' quella di creare Query mescolando variabili provenienti da un input esterno
direttamente nelle stringhe di query:

..
$validUser= mysql_query("Select id from
 autentificazioneUtenti 
where nomeUtente= {$_POST[nome]} and parolaChiave= {$_POST[passwd]}");
.. 

inutile dire che anche qui c'e' un serio problema di sicurezza, si puo' risolvere in diversi modi :

controllando la pagina che invia i dati( controllo semplice ma che non elimina del tutto i problemi )
utilizzando le funzioni htmlspecialchars() mysql_real_escape_string()[o funzioni simili relative al db in uso] nl2br() ,
un altro modo e' quello di usare Query parametrizzate tramite librerie scritte ad hoc.

4)MAI GET SENSIBILI (si offendono)
un errore che mi capita a volte di vedere, ma che mi fà rabbrividire e' quello di passare in GET dati sensibili

esempio

main.php?id=123&perm1=0&perm2=0

se a quel id corrisponde un utente, e io cambiassi la querystring otterrei facilmente privilegi amministrativi..

in ogni caso mai passare dati sensibili in GET o in POST utilizzare a questo scopo solo ed esclusivamente le Session.

5)MAI FIDARSI DEGLI INPUT

la regola basilare per evitare la maggiorparte degli errori e' mai fidarsi di qualsiasi input quindi prendete le giuste precauzioni
quando lavorate con $_GET $_POST $_COOKIES e anche prestate attenzione anche a cosa tirate su dal db..

[badaze]

Oltre a quello scritto da kentaromiura.

Si potrebbe aggiungere che bisogna limitare la lunghezza dell'input alla lunghezza del dato.

Se un dato ( tipo una password ) è lungo 10 conviene usare soltanto i dieci primi caratteri (non basta il maxchars del tag input).

$password = substr($_POST['psw'],0,10);

e non

$password = $_POST['psw'];

Di più, se un parametro di una query è numerico, metterlo tra apici.

ad esempio :

$query = "select pippo from pluto where id=$id";

se al posto di un numero metto una stringa, la query pianta. Mentre con

$query = "select pippo from pluto where id='$id'";

non pianterà.

[kuarl]

Originariamente inviato da badaze
Di più, se un parametro di una query è numerico, metterlo tra apici.

ad esempio :

$query = "select pippo from pluto where id=$id";

se al posto di un numero metto una stringa, la query pianta. Mentre con

$query = "select pippo from pluto where id='$id'";

non pianterà.

ma non darà neanche risultati... è meglio passare ai raggi x tutto ciò che viene dall'esterno prima di passarlo a mysql, se voglio un numero e non ho un numero a mysql quel dato non lo faccio arrivare. Non so' se capisci cosa intendo

[badaze]

Originariamente inviato da kuarl
ma non darà neanche risultati... è meglio passare ai raggi x tutto ciò che viene dall'esterno prima di passarlo a mysql, se voglio un numero e non ho un numero a mysql quel dato non lo faccio arrivare. Non so' se capisci cosa intendo

1 - Se metti degli apici ad una variabile numerica funziona.

2 - Hai ragione... ma per chi non fa i controlli, mettere gli apici è già qualcosa. Guarda l'allegato x vedere quello che succede ( ho trovato il sito a caso in meno di 5 minuti ). Ho cambiato un parametro numerico con un valora alfanumerico index.php?n=r

chissa forse con una sql injection gli distruggo il db....

[gm]

Aggiunta alle pillole in rilievo

[andr3a]

Originariamente inviato da badaze
Se un dato ( tipo una password ) è lungo 10 conviene usare soltanto i dieci primi caratteri (non basta il maxchars del tag input).

$password = substr($_POST['psw'],0,10);

non sono d'accordo, se la password deve essere di massimo 10 caratteri ed e' piu' di 10 caratteri significa che stanno gia' sbagliando o tentando attacchi.
Inoltre un' altra pratica comune e sbagliatissima e' proprio quella di non verificare l' esistenza dei dati, generando notice a non finire ...
Riassumendo:

$_POST['psw'] = isset($_POST['psw']) ? trim($_POST['psw']) : false;
$_POST['psw'] = $_POST['psw'] !== false && strlen($_POST['psw']) <= 10 ? $_POST['psw'] : false;

solo adesso posso fare la query nell' unico caso in cui
$_POST['psw'] !== false
altrimenti non ha senso nemmeno tentare di farla.

Originariamente inviato da badaze
Di più, se un parametro di una query è numerico, metterlo tra apici ... se al posto di un numero metto una stringa, la query pianta. Mentre con gli apici non pianterà.

altro errore comune ... che si rifa' al controllo necessario per ogni parametro.
Se un parametro deve essere numerico e in questo caso deve essere un intero si fa il cast, non si lascia a mysql e ad un paio di apici il compito di proteggerci poiche' significa che una banale sql injection potrebbe mandare tutto a buone donne.

quindi

$id = (int)$id; 
// oppure
$id = intval($id);

if($id > 0) { // ... la query

solo cosi' siamo certi che non ci saranno problemi e non con 2 semplici apici.

Se invece il campo non e' numerico usare le funzioni di escape del relativo database.


[editato]
aggiungo, sempre riguardo le password, che se queste sono hashate con md5, sha1 o altro e vengono memorizzate temporaneamente in sessioni o modificate prima di essere inviate tramite Flash o JavaScript, e' sempre meglio controllare che l' hash sia valido.


if(preg_match("/^[a-f0-9]{32}$/", $password)) {
.... ecco, ora posso andare avanti se e' md5
"/^[a-f0-9]{40}$/" per sha1
... etc etc

idem per tutte le altre password non hashate ... difficilmente una password puo' usare caratteri diversi da a-zA-Z0-9_-

quindi un controllo in preg puo' evitare tantissimiproblemi

[andr3a]

Originariamente inviato da badaze
2 - Hai ragione... ma per chi non fa i controlli, mettere gli apici è già qualcosa.

in una pillola come questa non deve esistere il caso "chi non fa controlli", altrimenti cosa l' ha scritta a fare ?

[gm]

BUMP