Miałem ostatnio okazję popatrzeć w kod jednej aplikacji, napisanej w PHP. Schemat działania był taki, że był główny "kontroler", który w parametrach przyjmował informacje o module. Kontroler wykonywał include modułu. I niby wszystko działało, gdyby nie kilka drobnych szczegółów. W dodatku te szczegóły powtarzają się w wielu różnych kawałach kodu, różnych autorów, więc warto o nich wspomnieć.
Nieskuteczne zabezpieczenie, czyli - myślenie (nie)boli
Szczegół pierwszy - funkcja str_replace nie działa in place. A to oznacza, że string (lub tablica) przekazana jako parametr funkcji nie jest modyfikowana. Innymi słowy - nie wystarczy wywołać funkcję, trzeba jeszcze przypisać do czegoś zwracaną wartość. Teoretycznie (gdyby nie drugi szczegół, o którym później) wywołanie tej funkcji oczyszcza dane wejściowe usuwając kombinacje znaków pozwalających na path traversal. W praktyce wywołanie to nie ma znaczenia.
Szczegół drugi - wywołanie funkcji str_replace miało na celu usunięcie "niebezpiecznych" ciągów znaków: .., ../ i /. Chodziło oczywiście o zabezpieczenie przed path traversal. Zagadka, co się stanie, gdy przekazany string będzie miał postać: ./.,katalog? Idąc od lewej fragment .. nie zostanie znaleziony, fragment ../ również nie zostanie znaleziony, znaleziony zostanie natomiast / i pojawi string przyjmie wartość ..,katalog. Gdzie tu path traversal? Jeszcze nigdzie, tylko, że później wołana jest funkcja explode, gdzie w roli separatora występuje (oczywiście) znak ,. Następnym krokiem jest wywołanie funkcji implode, gdzie łącznikiem jest (oczywiście) znak /, czyli ostatecznie mamy ../katalog. Ups...
Całość załatwiłaby prosta reguła walidacji oparta o regexp, coś w stylu ^[a-z]+,[a-z]+$. Poza tym oczyszczanie danych wejściowych wcale nie koniecznie jest dobrym pomysłem. Jeśli wartość parametru ma mieć format typu opisywanego przez powyższy regexp, to w przypadku, gdy takiej wartości nie ma, nie należy go oczyszczać, lecz po prostu (bezpiecznie) się wywalić. Dlaczego? Jeśli dany parametr w założeniu nie ma być modyfikowany przez użytkownika, to jakim cudem jest nieprawidłowy? Dlatego, że aplikacja jest błędna i w jakiś sposób na przykład generuje błędne linki? OK, ale to jest bug, który należy poprawić, a nie budować jego obejście. Dlatego, że użytkownik/intruz coś knuje? Po co ryzykować?
