Zákaz vkládání řetězců nebo alespoň ztížení

David Grudl · před 9 lety

Dibi je velmi bezpečný databázový layer, nicméně na vynalézavost některých programátorů nemá :-)

Občas narážím na podobný kód:

dibi::query("SELECT * FROM [table] WHERE name='$name'"); // namísto name=%s', $name);

Což samozřejmě nese riziko SQL injection. Uvažoval jsem, jak tomu zabránit. Jednou z možností by bylo zakázat jednoduché uvozovky a podporovat jen dvojité, protože na zápis name='" . $name . "' se narazí asi méně (ale možná se pletu). Případně použít úplně jiný znak.

Je mi jasné, že nejde jen tak přestat podporovat uvozovky. Buď by bylo možné je spojit s vyhozením E_WARNING, nebo přidat nějaký přepínač.

Co myslíte?

westrem · před 9 lety

No toto bude asi dost zakrsly problem. Zober si pripad:

dibi::query("SELECT * FROM [table] WHERE [type] = 'new'");

Tzn. ked niekto z nejakeho dovodu (napr. menej parsovania) da do query staticky string. V tomto pripade riziko nehrozi a nepodporovanie uvodzoviek by malo za nasledok (asi) vratenie chyby o zlej query.

Skorej by som sa priklonil k vyhadzovaniu tomu E_USER_WARNING, ze pozor v tej query moze byt bezpecnostne riziko spolu s niecim ako „Mali ste namysli toto?“

protože na zápis name=‚" . $name . "‘ se narazí asi méně (ale možná se pletu)

Odpovedal si si sam, na vynaliezavost proste len tak hocikto nema .. verim tomu, ze aj takyto zapis moze byt u niekoho uplne bezny.

muta · před 9 lety

no a co to kontrolovat na znak $ ten v mysql syntaxi neni. Timhle by mi to prislo nejbezpecnejsi. A jsem pro vyhazovani vyjimek :)

Petr

paranoiq · před 9 lety

no a co to kontrolovat na znak $ ten v mysql syntaxi neni.

WTF? O_o . k dosazení proměnných dojde dřív než se ti řetězec dostane do ruky

Jednou z možností by bylo zakázat jednoduché uvozovky a podporovat jen dvojité, protože na zápis name='" . $name . "' se narazí asi méně

nechápu. tohle name='" . $name . "' a tohle name='$name' je přeci totéž

chtěl by jsi snad v rámci bezpečnosti zavést možnost pro tohle?:

$name = '"; DROP TABLE [users]; --';
dibi::query("SELECT * FROM [table] WHERE name=\"$name\"");

uživatelé i útočníci jsou vynalézaví. ať bude oddělovač řetězců v dibi query jakýkoliv, vždy jej bude možné překonat. jedinou možností je konstantní řetězce vůbec nepodporovat

a varování povede akorát tak k tomu, že si ho ti, kterým je nejvíce určeno, vypnou :[

westrem · před 9 lety

muta napsal(a):

no a co to kontrolovat na znak $ ten v mysql syntaxi neni. Timhle by mi to prislo nejbezpecnejsi. A jsem pro vyhazovani vyjimek :)

Petr

Presne ako pise paranoiq, vyhodnotenie stringu sa predsa spravi skorej ako ho parser od dibi zacne spracovavat ;)

paranoig napsal

uživatelé i útočníci jsou vynalézaví. ať bude oddělovač řetězců v dibi query jakýkoliv, vždy jej bude možné překonat. jedinou možností je konstantní řetězce vůbec nepodporovat

Tiez som rozmyslal nad tym, ze napisem toto, ale nebol som si isty ci by to kompatibilne nemohlo robit nejake problemy. Ale myslim si, ze je to way to go, pretoze ak chce niekto raw retazce tak nech vyuziva ::nativeQuery a nie ::query.

a varování povede akorát tak k tomu, že si ho ti, kterým je nejvíce určeno, vypnou :[

To je mozne, ale v tom pripade je to uz nanajvys blbost a starost toho cloveka, nicmene viem si predstavit situaciu, ked niekto pise kod ako ukazal David:

dibi::query("SELECT * FROM [table] WHERE name='$name'"); // namísto name=%s', $name);

S tym, ze si mysli, ze ked pouziva „obalku“ dibi tak je chraneny no v skutocnosti ostal pri starom zapise SQLka a takto si tam tu bezp. dieru privolava sam. V tomto pripade by mu ten error pomohol a ukazal spravnu cestu, nemyslis?

paranoiq · před 9 lety

dejme tomu, že by to vyhazovalo warning. při zápisu konstantního řetězce chybu jednoduše umlčím. šak vím co dělám, žé:

@dibi::query("SELECT * FROM [table] WHERE name='default'");

vše v pořádku.

za měsíc někdo udělá úpravu:

@dibi::query("SELECT * FROM [table] WHERE name='$name'");

zavináč přehlédne. a kde to jsme? :[

zakázat uvozovky a použít nativeQuery nelze. to by vedlo k ještě horším věcem. někteří by si na to zvykli a začali to běžně používat a ochrana proti SQLi by se naopak zhoršila

myslím, že celý tenhle problém nemá smysl vůbec řešit (jedině pořádným upozorněním v dokumentaci), protože spolehlivé řešení není a zakázání řetězců v query by bylo příliš radikální. pokud se někdo chce střelit do nohy, tak prosím…

paranoiq · před 9 lety

možná by mělo smysl, aby dibi::query vyhodila výjimku, pokud dostane ke zpracování řetězec obsahující více než jeden dotaz – to může být jasným příznakem toho, že došlo k SQLi. detekce by nemusela být tak složitá, ale musela by se vzít v úvahu třeba změna delimiteru

vzhledem k tomu, že dibi nepodporuje vícenásobné resultsety, tak asi k pokládání více dotazů jedním voláním query příliš důvodů není. případně by se mohla přidat metoda multiQuery

David Grudl · před 9 lety

paranoiq napsal(a):

možná by mělo smysl, aby dibi::query vyhodila výjimku, pokud dostane ke zpracování řetězec obsahující více než jeden dotaz – to může být jasným příznakem toho, že došlo k SQLi. detekce by nemusela být tak složitá, ale musela by se vzít v úvahu třeba změna delimiteru

K výjimce obvykle dojde, protože snad kromě SQLite nikdo vícenásobné dotazy nepodporuje. Ale injection se netýká jen vícenásobných dotazů.

Oznámení

Zákaz vkládání řetězců nebo alespoň ztížení