Newsgroup: it.comp.www.php
Titolo: SQL Injection

Supponendo di avere una pagina test.php nella quale sono contenute le
istruzioni
$id = $_GET['id'];
mysql_query("SELECT * FROM table WHERE id=$id");
Se un simpaticone passasse la continuazione di una stringa SQL ( ad es
"; mysql_query("DELETE FROM table
potrebbe, in teoria, danneggiare db, alterare record ecc.
Per proteggersi da questo sabotaggio, basterebbe controllare l'  $id
if (!eregi("^[0-9],$_GET['id']) {
       die("formato non corretto");
}
$new_id = $_GET['id'];
e
$real_id = addslashes($id);
in questo modo prima della query? O il procedimento è diverso?

 mysql_query("SELECT * FROM table WHERE id=$id");
In breve, come linee guida:
prima di eseguire la query controlli che il dato sia di tipo numerico 
(la funzione che ti viene in aiuto è is_numeric), lo forzi a essere in 
ogni caso numerico (la funzione intval fa al caso tuo), cambi la query 
facendo:
id='$id'
invece di:
id=$id
e infine usi la funzione di escaping mysql_real_escape_string per 
evitare che il simpaticone faccia porcherie.
Non ne sono sicuro (diciamo che la mia certezza su questo argomento è 
al 10% :-) ), comunque credo che in mySQL non sia possibile eseguire 
tramite php query multiple, o mi sbaglio? Se coì fosse il simpaticone 
per tua fortuna non potrebbe fare poi così tanti casini.
Se qualcuno comunque è più informato riguardo questa cosa che ho detto, 
io son tutto orecchi!! :-)
--
rob4you
www.outofmind.eu

Per proteggersi da questo sabotaggio, basterebbe controllare l'  $id
Oppure forzarlo a tipo intero:
	$id = (int)$_REQUEST['id'];
Leonardo
-- 
Geordie - he's a man, there's little doot about't,
He's done all he can, wha' can do withoot't?
Doon there cam a blade, linkin' like me lordie,
He wad drive a trade, a' the loom of Geordie.

metodino facile e completo:
(da chiamare SEMPRE prima di una query)
function parse_incoming() {
if ( is_array($_GET) )
		{
			while( list($k, $v) = each($_GET) )
			{
				if ( is_array($_GET[$k]) )
				{
					while( list($k2, $v2) = each($_GET[$k]) )
					{
						$this->input[ $this->parse_clean_key($k) ][
$this->parse_clean_key($k2) ] = $this->parse_clean_value($v2);
//$this->input[chiave][valore] lo puoi sostituire con un altro tipo di
array. Per generalizzare sarebbe meglio prendere tutti gli input e
metterli in questo array "input" con chiave = nome del campo form e
valore il valore che arriva dal form)
					}
				}
				else
				{
					$this->input[ $this->parse_clean_key($k) ] =
$this->parse_clean_value($v);
				}
			}
		}
		//----------------------------------------
		// Sovrascrive i GET con i dati POST (evita il passaggio di variabili
dal GET e li mette in POST, più sicuro!
		//----------------------------------------
		if ( is_array($_POST) )
		{
			while( list($k, $v) = each($_POST) )
			{
				if ( is_array($_POST[$k]) )
				{
					while( list($k2, $v2) = each($_POST[$k]) )
					{
						$this->input[ $this->parse_clean_key($k) ][
$this->parse_clean_key($k2) ] = $this->parse_clean_value($v2);
					}
				}
				else
				{
					$this->input[ $this->parse_clean_key($k) ] =
$this->parse_clean_value($v);
				}
			}
		}
}
function parse_clean_key($key)
	{
		if ($key == "")
		{
			return "";
		}
		$key = htmlspecialchars(urldecode($key));
		$key = preg_replace( "/\.\./"           , ""  , $key );
		$key = preg_replace( "/\_\_(.+?)\_\_/"  , ""  , $key );
		$key = preg_replace( "/^([\w\.\-\_]+)$/", "$1", $key );
		return $key;
	}
function parse_clean_value($val)
	{
		if ($val == "")
		{
			return "";
		}
		$val = str_replace( "&#032;", " ", $val );
		if ( $this->vars['strip_space_chr'] ) //sempre true sarebbe ideale
		{
			$val = str_replace( chr(0xCA), "", $val );  //Rimuove i ritorni a
capo messi come caratteri esadecimali o codici
		}
		$val = str_replace( "&"            , "&amp;"         , $val );
		$val = str_replace( "<!--"         , "&#60;&#33;--"  , $val );
		$val = str_replace( "-->"          , "--&#62;"       , $val );
		$val = preg_replace( "/<script/i"  , "&#60;script"   , $val );
		$val = str_replace( ">"            , "&gt;"          , $val );
		$val = str_replace( "<"            , "&lt;"          , $val );
		$val = str_replace( "\""           , "&quot;"        , $val );
		$val = preg_replace( "/\n/"        , "<br />"        , $val ); //
Convert literal newlines
		$val = preg_replace( "/\\\$/"      , "&#036;"        , $val );
		$val = preg_replace( "/\r/"        , ""              , $val ); //
Remove literal carriage returns
		$val = str_replace( "!"            , "&#33;"         , $val );
		$val = str_replace( "'"            , "&#39;"         , $val ); //
IMPORTANTE: Aiuta a creare una query pulita.
		// Verifica che i caratteri UNICODE siano "accettabili"
		if ( $this->allow_unicode ) //puoi metterla sempre su true o su false,
secondo necessità
		{
			$val = preg_replace("/&amp;#([0-9]+);/s", "&#\\1;", $val );
		}
		// Esegue lo strip degli slash se non è già stato fatto.
		if ( $this->get_magic_quotes ) //come sopra, sempre true o sempre false
		{
			$val = stripslashes($val);
		}
		// Swappa i backslash inseriti dall'utente
		$val = preg_replace( "/\\\(?!&amp;#|\?#)/", "&#092;", $val );
		return $val;
	}
rob4you , in data 19/12/2006 16:07 :


               dal GET e li mette in POST, più sicuro!
da cosa deriverebbe questa maggiore sicurezza?
p.s.
al posto tuo darei una bella lettura qui:
http://pear.php.net/manual/en/standards.php

 
....MEGA CUT


hk3project , in data 19/12/2006 17:05 :

ihhh che barba :p
Ecco il quote giusto :p
Beh serve che così tutto il GET lo metti obbligatoriamente nel POST
evitando di doverlo controllare per eventuali codici o istruzioni
"maligne", controllando tutto nel POST...se ad esempio come GET ti passo
un "include" ben formattato e tu non mi controlli i GET posso far
includere un codice maligno tipo una shellroot o simili ;)

 
http://pear.php.net/manual/en/standards.php
Se ti riferisci al fatto che ti va a capo il commento è normale...o
meglio...è il programma che uso per rispondere che fa sta schifezza...se
vuoi ti riposto il tutto correttamente ;)
Di nuovo scusate l' OT

 vuoi ti riposto il tutto correttamente ;)
no, grazie preferisco vivere :)

     for ($i=0; $i<100;$i++=
                             ^^
e questo è il copiaincolla del "refuso" :)


Oddio ti prego non venire a parlare a me di manutenibilità del codice!
A parte le disquisizioni sul fatto che sia o meno "conforme allo
standard PEAR" direi che è funzionale e funzionante no?
Quindi tralasciamo inutili polemiche su "ci vuole lo spazio tra il
termine dell' IF e la parentesi graffa....altrimenti dovrei mettere
tutti i copyright del caso nel codice per seguire le indicazioni di PEAR ;)

Piso ha scritto:

cosa mi impedirebbe di "passarti un include" tramite post?

Piso ha scritto:

non ho nessuna intenzione di fare "inutili polemiche" ma è innegabile che il codice
che hai postato (come forma) sia da brrrrivido :)

 cosa mi impedirebbe di "passarti un include" tramite post?
Assolutamente nulla, ma col mio metodo TUTTO ciò che arriva da GET viene
spostato in POST e quindi ti basta controllare i POST anzichè SIA i GET
che i POST ;)
Non è difficile no?Altrimenti, sempre che tu voglia usare il mio metodo
ma senza occuparti dei GET (o occupandotene in separata sede) devi
rifare tutto il giro un'altra volta e trattare poi sia i GET che i
POST...due giri, due pezzi di codice da manutenere, due fonti di
possibili errori, malfunzionamenti, bachi e hacking/cracking :D
(sarà da brrrrivido ma funziona...la prossima volta ti metto tutti i
copyright del caso e i commenti :| )

 copyright del caso e i commenti :| )
   guarda che il "brrrrivido" era per la forma ...
il codice manco l'ho letto ...
   così ... al volo invece di fare sedicimila (:p) str_replace perchè non ne fai uno solo,
oppure usi strtr ...

 function parse_incoming() {
questa funzione non e' ottimizzata e in piu' usa ancora il while/list
per ciclare gli array (di cui non c'e' piu' bisogno da anni)
ecco una versione senza ridodanze e con l'uso di foreach:
function parse_incoming($array) {
    if (is_array($array)) {
        foreach ($array as $k => $v) {
            if (is_array($v)) {
                foreach ($v as $kk => $vv) {
$this->input[$this->parse_clean_key($k)][$this->parse_clean_key($kk)] =
$this->parse_clean_value($vv);
                }
            } else {
                $this->input[$this->parse_clean_key($k)] =
$this->parse_clean_value($v);
            }
        }
    }
}
parse_incoming($_GET);
parse_incoming($_POST);
ovviamente in questo modo (ma anche nel tuo) i valori in $_POST
sovrascrivono quelli in $_GET, che e' un modo per far rientrare dalla
finestra le register_global ad on. ovviamente il codice andrebbe
modificato perche' cio' non accada
c'e' un altro limite, ovvero il fatto che prevedi che gli array passati
alla funzione abbiano solo due dimensioni, cosa che non e' detta
ovviamente, la funzione andrebbe modificata di conseguenza e resa
ricorsiva (ma questo lo lascio a te)

 function parse_incoming() {
questa funzione non e' ottimizzata e in piu' usa ancora il while/list
per ciclare gli array (di cui non c'e' piu' bisogno da anni)
ecco una versione senza ridodanze e con l'uso di foreach:
function parse_incoming($array) {
    if (is_array($array)) {
        foreach ($array as $k => $v) {
            if (is_array($v)) {
                foreach ($v as $kk => $vv) {
$this->input[$this->parse_clean_key($k)][$this->parse_clean_key($kk)] =
$this->parse_clean_value($vv);
                }
            } else {
                $this->input[$this->parse_clean_key($k)] =
$this->parse_clean_value($v);
            }
        }
    }
}
parse_incoming($_GET);


 
Beh per quel che aveva chiesto l'utente dovrebbe essere ben più che
sufficiente *-)

 sufficiente *-)
cio' che e' sufficiente non sempre e' corretto o pulito, chi ne sa di
meno impara dagli esempi: un esempio fatto male vuol dire insegnare
cattiva programmazione

T_PAAMAYIM_NEKUDOTAYM , in data 19/12/2006 21:43 :

Cito tutto e mi astengo da ogni commento che è meglio....

ok, grazie a tutti per l'aiuto, ma non mi sembra che sia il caso di
iniziare una luuuuunga discussione sul codice pulito e rigorosamente
adernete agli standard...
a me basta capire cosa devo fare, dove sbaglio o il meccanismo e poi
copio, incollo, taglio modifico, elimino, scrivo, ritaglio, rimodifico
e alla fine scopro che ho sbagliato e riparto da capo :D
a presto gg

 Cito tutto e mi astengo da ogni commento che è meglio....
se non sei d'accordo puoi benissimo dirlo e ribattere, si chiamano
gruppi di _discussione_ apposta

T_PAAMAYIM_NEKUDOTAYM , in data 20/12/2006 21:43 :

[OT]
Voglio solo evitare dei flame che non hanno alcun senso, l'utente ha
giustamente detto "grazie a tutti ma NON mi sembra il caso di disquisire
a lungo su metodi corretti di formattare il codice eccetera" che guarda
caso è proprio quel che ho detto IO...a lui bastava un codice
funzionante per capire come si fa e poi ci smanetta sopra e trova una
soluzione adatta alle sue esigenze ;)
Tanto per chiarire il mio codice era VOLUTAMENTE ridondante perchè se
deve trattare diversamente GET e PUT col mio lo può fare, col tuo no ;)
[Fine OT]

 caso è proprio quel che ho detto IO...a lui bastava un codice
Dalle mie parti si dice che a far una cosa fatta bene o fatta male si 
impiega il solito tempo.
Io sono contrario all'etichetta: gira, ok va bene[*]; nel momento 
risolve la soluzione ma se c'e' la possibilità di 
imparare/apprendere/confrontarsi su qualcosa di più perchè rifiutarla?
Anzi spesso e volentieri thread interessanti vengono 
accantonati...sicuramente attraverso la discussione abbiamo tutti (e 
sottolineo _tutti_) da imparare.
[*] Sicuramente la formattazione non influenza il funzionamento del 
codice ma rende più semplice una successiva analisi/correzione
	Francesco

     Francesco
Prescindendo che la formattazione del testo me l'ha fatta Zend Studio
(mica il primo programmino che arriva sul mercato ;) ) effettivamente è
vero, nel caso in cui però la formattazione sia completamente sballata.
Mi risulta che l'unico appunto fattibile sia l'andare a capo erratamente
nei costrutti
{
... } else {
...
}
mentre tutto il resto è direi "perfetto"...o no?Errore veniale, credo
che TUTTI ogni tanto si commetta di questi errorini (tanto per
dire...andrebbero usati gli spazi e non le tabulazioni per identare ;) )
COMUNQUEEEEEEEEEEEEEEEeeeeeee....problema chiuso, ok?
Come detto i prossimi script li farò RIGOROSAMENTE seguendo le
indicazioni di PEAR e che nessuno osi lamentarsi eh!! :p

 COMUNQUEEEEEEEEEEEEEEEeeeeeee....problema chiuso, ok?
Tengo solo a precisare che non ho assolutamente fatto riferimento al 
thread in questione, al codice di tizio o caio, era solamente un mio 
pensiero, espresso ad alta voce
	Francesco

 attieniti alle regole PEAR, è ridondante" eccetera...no?
io non ho fatto nessuna osservazione sulla formattazione del codice
ti ho detto cosa non andava nel codice e in piu' ho riscritto la
funzione in maniera piu' strutturata, il che mi sembra ben piu' utile
che essere cerimoniosi
se poi ti offendi perche' ti senti bistrattato ti consiglio di
abituarti alla sostanza delle cose, e non alla forma

Tornando IT, mi viene in mente...per evitare un PURO sql injection
potrebbe essere utile anche "epurare" l'input e bloccare l'utente da
altre azioni controllando appunto gli input...del tipo...
se come GET infilo un %20UNION%20SELECT%20password eccetera nel punto
giusto (variabile in base alla conoscenza o meno del sorgente e dei dati
che si ricavano a tentoni) e non ho epurato l'input correttamente (cosa
che diventa molto probabile in progetti "grossi") allora tutti i modi
descritti prima potrebbero NON funzionare.
Spero di aver fatto capire bene il concetto.
Soluzione rapida è controlalre che nella stringa di GET (tutta la
stringa, non nelle singole variabili) NON vi sia presente la frase
"union select" e derivate e se c'è mostrare un errore. La stessa cosa si
può fare coi POST ma così a occhio è più difficile passare una
istruzione come quella in una variabile POST, essendo esse generalmente
molto più controllabili (generalmente).
Non so se può essere d'aiuto questa considerazione, nel caso proviamo a
buttare giù uno scrippettino o una funzione che faccia questi controlli :)

 attieniti alle regole PEAR, è ridondante" eccetera...no?
se io scrivo:
"p.s.
  al posto tuo darei una bella lettura qui:
 
http://pear.php.net/manual/en/standards.php
"
e tu leggi: "attieniti alle regole PEAR"
evidentemente c'è qualche problema di comunicazione