Mingle Forum to jedna z najpopularniejszych wtyczek do WordPressa służących do tworzenia forów internetowych – do tej pory pobrało ją ponad 120 tysięcy użytkowników. Ja sam z powodzeniem stosuję ją do obsługi użytkowników mojej wtyczki TradeMatik. Działa świetnie, jednak nie ma rzeczy bezbłędnych – wczoraj wykryto poważną lukę bezpieczeństwa umożliwiającą na wstrzyknięcie dowolnego polecenia do bazy MySQL (tzw. sql injection).
Każdemu się zdarza. Autor wtyczki szybko ją naprawił i dziś każdy może sobie zaktualizować do wersji 1.0.31.1 i korzystać bez obaw. A skoro luka została już naprawiona, zobaczmy co było nie tak. Mam nadzieję, że to będzie dobra lekcja dla nasz wszystkich i sami nigdy nie popełnimy takiego błędu.
Jak było?
Cały problem znajdował się w pliku wpf-insert.php. Plik ten potrafi odbierać dane przesłane za pomocą formularza, a następnie wprowadza przesłane wartości do bazy danych. Dla tych, którzy wiedzą co to jest sql injection brzmi niebezpiecznie, prawda? I tak właśnie było.
Ta linijka:
$edit_post_id = $_POST['edit_post_id'];Odbierała zmienną 'edit_post_id’ wysłaną metodą POST i przypisywała do zmiennej lokalnej o tej samej nazwie. Co dalej się z nią działo?
//SECURITY FIX NEEDED
$sql = ("UPDATE $mingleforum->t_posts SET text = '$content',
subject = '$subject' WHERE id = $edit_post_id");
$wpdb->query($wpdb->prepare($sql));…otóż to. Zwróćmy uwagę na 1. linijkę zacytowanego kodu – autor zamieścił tam komentarz informujący, że poniższy kod wymaga poprawy z powodów bezpieczeństwa :) Sam widział, że coś jest nie tak, pozostawił to jednak nienaprawione, a co więcej poinformował wszystkich przeglądających jego kod:
„Jeśli chcielibyście się do mnie włamać, wchodźcie tędy.”
Być może tak właśnie odkrywca błędu natknął się na niego? Kto wie.
Na czym jednak błąd polega?
Na nieprawidłowym użyciu metody prepare(). Rzeczywiście służy ona do zabezpieczenia zapytań sql przed wstrzyknięciem do nich złośliwego kodu, jednak w takiej formie, jak ją użyto jest kompletnie bezużyteczna. prepare() przede wszystkim zamienia apostrofy i cudzysłowy na ich bezpieczną formę (zamiast ’ wstawia \’). I co równie ważnie, a może nawet ważniejsze: dodaje na początku i końcu danych apostrofy, by na pewno traktowane one były jak pojedyncze ciągi znaków.
Tyle, że musi ona wiedzieć, które z nich powinna zmienić. Wyobraźmy sobie, że w zmiennej edit_post_id prześlemy taki ciąg znaków:
12;DROP TABLE wp_posts;Po podstawieniu tego w zmiennej $sql całe zapytanie będzie wyglądało w całości:
"UPDATE $mingleforum->t_posts SET text = '$content',
subject = '$subject' WHERE id =12;DROP TABLE wp_posts;"Co się stanie? Zostanie wykonany warunek WHERE=12, średnik po nim oznacza, że jest to koniec jednego polecenia SQL i zaczyna się następne. Następne natomiast usuwa tabelę z wszystkimi wpisami na naszym blogu*. A to już nie jest fajne.
Autor wtyczki próbował tutaj zastosować na zmiennej $sql metodę prepare(). Nic to jednak nie da. Przypomnijmy, że bierze ona otrzymaną zmienną, dodaje na początku i końcu cudzysłowy „wstrzykiwanych” wartości i „zabezpiecza” odpowiednie apostrofy i cudzysłowy w środku wartości.
Co jednak znaczy odpowiednie? Skąd metoda ma wiedzieć które elementy w ciągu $sql są wartościami? Nie potrafi tego rozpoznać, więc zostawia całą zmienną taką, jaka ona jest. Wstrzyknięcie więc się uda.
Jak powinno być?
Metoda prepare() powinna zostać użyta jeszcze przed podstawieniem do niej przez nas niebezpiecznych zmiennych. Ma ona postać:
prepare('zapytanie z %s', array('niebezpieczny ciąg'));Wszelkie niebezpieczne ciągi (pobrane np z formularza wysyłanego przez użytkownika) podajemy jako drugi parametr w tablicy (możemy tak podać od razu wiele ciągów do zabezpieczenia). Zostaną one objęte w cudzysłowy/apostrofy jeśli jeszcze nie są, a apostrofy/cudzysłowy w środku ciągu zostaną zabezpieczone przez backshlash \. Następnie po takim zabezpieczeniu zostaną podstawione do pierwszego parametru w miejsca gdzie występują ciągi „%s”**.
Następnie możemy już na takim ciągu bezpiecznie wykonać metodę query(). Całość w prawidłowej formie więc wygląda tak:
$sql = $wpdb->prepare("UPDATE $mingleforum->t_posts
SET text = '$content',subject = '$subject' WHERE id = %s",
array($edit_post_id));
$wpdb->query($sql);I teraz jest prawidłowo. Zmienna $sql przed wykonaniem jej przez query() przyjmuje postać (jeśli wstrzykniemy do niej niebezpieczny kod pokazany powyżej):
"UPDATE $mingleforum->t_posts SET text = '$content',
subject = '$subject' WHERE id ='12;DROP TABLE wp_posts;'"Sprawdzany jest warunek czy pole „id” w bazie ma dziwną wartość „12;DROP TABLE wp_posts;”. Takiej wartości w tym polu nie ma na pewno, więc polecenie nie zostanie wykonane.
I wszystko nadal pozostaje bezpieczne.
Jak wspomniałem na początku, wtyczka ma już to naprawione, tak więc wszystkich, którzy jeszcze jej nie zaktualizowali, usilnie namawiam by jednak to zrobili.
A tych, którzy będą teraz próbować wykorzystać tę lukę… cóż – próbujcie :) Na moim TradeMatik jest już naprawiona wersja. A jeśli znajdziecie gdzieś wtyczkę w starszej wersji, dodam, że nie napisałem wszystkiego. W innej części kodu wspomnianego pliku jest wpisane dodatkowe zabezpieczenie. Osoby biegłe z łatwością je obejdą. Jednak wszyscy wannabe script kiddies pozostaną na szczęście tylko wannabe. Ja ich z tego stanu wyprowadzać nie mam zamiaru, więc tylko na tym fragmencie kodu, który potrzebował naprawy się skupiłem ;)
___
*) tak, wiem, że nie zawsze się to uda, ale jest to tylko ilustracja jak przebiega sql injection. **) lub „%d” dla ciągów, które mają być traktowane jako liczby a nie zmienne typu string.
Comments