Słowo wstępu - to nie jest problem
- Ponieważ PHP jest zwalony, to da się definiować stałe globalne w runtimie. Również stałe które są używane w wewnętrznych algorytmach.
- W PHP jest też konwencja że sporo feature'ów ma flagi, więc często jak się chce sprawdzić czy jakiś extension lub feature języka już jest, to się sprawdza
function_exists('mb_strlen')
albodefined(PREG_UNATMCHED_AS_NULL)
. Jeśli constant istnieje - to można użyć feature'a.
Mój use-case - to nie jest problem
Piszę bibliotekę która ma wspierać PHP 7.1, 7.2, 7.3, 7.4 oraz 8.0 i 8.1. Tak, wiem że 7.3 i starsze są out of life - w projektach ich nie używam, ale to jest biblioteka więc wspieram.
W PHP 7.3 doszedł constant PREG_UNAMATCHED_AS_NULL
, który zwiększa trochę wydajność. To znaczy że biblioteka na nowszych wersjach PHP może dodać jakieś ulepszenie, a na starszych po prostu nie.
Kod działa tak:
function moja_funkcja() {
if (defined('PREG_UNMATCHED_AS_NULL')) { // const jest, jesteśmy na PHP 7.3+
return preg_match($p, $s, PREG_UNMATCHED_AS_NULL | PREG_CAPTURE_OFFSET); // optymalizacja
}
return preg_match($p, $s, PREG_CAPTURE_OFFSET); // stasza implementacja
}
Stałe PREG_UNMATCHED_AS_NULL
oraz PREG_CAPTURE_OFFSET
są systemowe, z modułu PCRE. Jedna z nich jest od początku, druga była dodana w PHP 7.3
Atak "constant override" - to nie jest problem
Tylko że w PHP da się dodać constant, jeśli nie istnieje! :O
function hakuj() {
define(PREG_UNMATCHED_AS_NULL, 2);
}
hakuj();
function moja_funkcja() {
if (defined('PREG_UNMATCHED_AS_NULL')) { // const jest, nawet na starszych PHP teraz
return preg_match($p, $s, PREG_UNMATCHED_AS_NULL | PREG_CAPTURE_OFFSET); // optymalizacja
}
return preg_match($p, $s, PREG_CAPTURE_OFFSET); // stasza implementacja
}
A to znaczy, że jeśli ktoś używa tej biblioteki na starszej wersji PHP, np 7.1 albo 7.2, na której nie ma ani funkcjonalności ani flagi PREG_UNMATCHED_AS_NULL
, to on jest w stanie na takim systemie dodać ten const PREG_UNMATCHED_AS_NULL
. PHP o nim nie wie. I wtedy, if if (defined('PREG_UNMATCHED_AS_NULL'))
, i zostanie wywołany kawałek kodu z optymalizacją, mimo że nie powinien.
Ergo, nie mogę użyć if (defined('PREG_UNMATCHED_AS_NULL'))
do sprawdzenia czy feature istnieje, bo można nadpisać po prostu const. Muszę użyć innego rozwiązania, np takiego:
function moja_funkcja() {
if (version_compare(PHP_VERSION, '7.3.0', '>=')) { // na pewno jesteśmy na PHP 7.3+
return preg_match($p, $s, PREG_UNMATCHED_AS_NULL | PREG_CAPTURE_OFFSET); // optymalizacja
}
return preg_match($p, $s, PREG_CAPTURE_OFFSET); // stasza implementacja
}
Wtedy hacker może sobie nadpisywać consty do woli. Super. Pierwszy problem rozwiązany.
Teraz jak to przetestować? - to nie jest problem
No oczywiście, bardzo prosto. Napisać test który deklaruje nową stałą, i sprawdza czy leci warning z użya biblioteki
public function constantOverrideAttack() {
// given
define('PREG_UNMATCHED_AS_NULL', 5); // na PHP 7.1 lub 7.2 to doda nieistniejący const
// when
moja_funkcja(); // tutaj leci warning, bo const jest, ale funkcjonalności nie ma
}
I to jest spoko test.
No więc do jasnej anielki co ten TomRiddle chce - TO JEST PROBLEM! TO CHCĘ NAPRAWIĆ
Ano problem jest taki, że tego consta z tego testu się nie da usunąć po teście. A to znaczy że inne testy, które się wykonają po nim też go będą miały, a to znaczy że testy nie są odseparowane od siebie, a muszą być.
Pomysły jakie mam.
- Zhackować PhpUnit żeby odpalał ten konkretny test jako ostatni. Zadziała tak długo jak będzie jeden, bo jak będę chciał więcej to klops.
- Odpalać dwa testy w osobnych procesach, tak żeby nie było zdefiniowanej tej stałej, ale to spowolni testy znacznie z 1ms do jakichś 2-3 sekund.
- Użyć extensiona "runkit" który może usunąć stałą, ale to ma szereg wad: po pierwsze nie da się łatwo zainstalować runkit na Github Actions, ale to bym ogarnął; ciężko jest zainstalować runkit zarówno na windowsie jak i na linuxie bo to nie jest standardowy extension, i trzeba go specjalnie budować ze źródeł. Dużo zachodu. Jak kontrybutor ściągnie to prędzej się pochlasta niż sobie zbuduje ten extension (bo on np potrzebuje Visual studio redistributables co ma 6gb i nie instaluje się go wcale tak prosto).
Byłoby spoko gdyby dało się zredefiniować stałą, wtedy ustawiłbym ją z powrotem na null
i inne testy byłyby safe. Ale niestety nie da się, pierwsza wartość zostaje.
Najbardziej się skłaniam ku opcji 2, ale to jest wolne strasznie.
O czym myślałem i czego nie zrobię - to nie jest problem
- Nie użyję żadnych linterów/checkerów/stylechecków do wykrywania czy użyłem
defined('PREG_UNMATCHED_AS_NULL')
, bo to mniej rzetelne rozwiązanie niż testy. - Nie wydzielę kawałka kodu do serwisu który potem będzie zmockowany, bo tak czy tak jego też musiałbym przetestować.