Czego nauczyłem się, audytując własny kod tak, jakby był cudzy

W jeden dzień zamknąłem sześć błędów bezpieczeństwa w API transkrypcji, które działało od miesięcy. Żadnego nie znalazł test. Testy świeciły na zielono przed i po. Nie zmieniło się narzędzie: zmieniło się to, że usiadłem do czytania własnego kodu z pytaniem, jak bym go zepsuł, gdybym chciał, zamiast pytać, czy robi to, co zaplanowałem.

Ta różnica wygląda na kwestię nastawienia. Jest kwestią metody i widać to po rodzaju błędów, które wychodzą.

Test sprawdza ścieżkę, którą sobie wyobraziłeś

Test pisze się od przypadku użycia: rozsądne dane, rozsądne wyjście. Atakujący nie podaje rozsądnych danych. Podaje pustkę, zero, wartość ujemną, ogromną. Takie wartości rzadko są w zestawie testów, bo kiedy piszesz test, nie przychodzą ci do głowy, a kiedy przychodzą, to już widziałeś buga.

Najwyraźniejszym przypadkiem była pusta lista ACL. Wyszukiwarka do odzyskiwania dostawała listę identyfikatorów, do których użytkownik ma dostęp, i filtrowała po niej korpus. Jeśli lista przychodziła pusta, kod interpretował ją jako "nie ma filtru do zastosowania", zamiast jako "ten użytkownik nie ma dostępu do niczego". Zwracał cały korpus.

To był fail open: domyślną reakcją na brak uprawnień było pozwolenie na wszystko. Żaden test szczęśliwej ścieżki tego nie dotknie, bo na szczęśliwej ścieżce lista zawsze coś zawiera.

Od tamtej pory przeglądam wszystkie filtry autoryzacji z tym samym pytaniem: co to robi, jeśli kolekcja jest pusta? Nie "co robi, jeśli użytkownik nie ma uprawnień", bo to zwykle jest dobrze pokryte, tylko co robi z pustką.

Kategorie powtarzają się bardziej, niż się wydaje

Znaleziska prawie zawsze układają się w te same rodziny.

• Uprawnienia nieegzekwowane tam, gdzie trzeba. Endpoint eksportu, który pozwalał wyciągać dane bez pytania o uprawnienie do zarządzania strukturą. Kaskadowe usuwanie w domenie, które sprawdzało uprawnienie raz na początku, zamiast sprawdzać uprawnienie do twardego usunięcia każdego dotkniętego elementu. Kaskada to klasyczne miejsce: autoryzujesz operację, a potem wewnętrzna pętla robi rzeczy, których nikt nie autoryzował.
• Limity, których nie ma. Listy administracyjne przekazywały do zapytania limit prosto z wejścia, bez sufitu. Duża wartość ze skryptu materializowała całą tabelę w pamięci workera. Poprawka mieści się w jednej linii, Math.min(Math.max(limit, 1), 1000), ale jej nie było, bo w UI limit zawsze wynosił dziesięć.
• Stan sprawdzany raz i nigdy więcej. authenticateApiKey walidował hash, wygaśnięcie i flagę isActive klucza. Ale wiersz klucza przeżywa stan konta właściciela: jeśli rekord rozliczeniowy jest czyszczony przy wyrejestrowaniu kogoś, nikt nie zdejmuje tej flagi i klucz dalej uwierzytelnia. Teraz funkcja sprawdza ponownie, czy właściciel nadal ma rekord rozliczeniowy, w jednym miejscu, żeby wszystkie endpointy uwierzytelniające kluczem odziedziczyły to, zamiast każdy musiał o tym pamiętać.
• Renderowana treść z zewnątrz. Tu jest to, co zaskoczyło mnie najbardziej.

CSS też jest wstrzyknięciem

Sanityzator markdownu pozwalał na dowolne atrybuty style i class w div, span i svg. Łańcuch idzie od raw przez sanitize do harden i żaden etap nie zagląda do środka CSS-a: biblioteka sanityzująca traktuje style jak nieprzezroczysty ciąg, a warstwa hardeningu przepisuje tylko URL-e w a i img. Styl docierał nietknięty do finalnego renderu.

Myślałem o napisaniu parsera CSS z białą listą właściwości. Nie zrobiłem tego: sprawdziłem, że żaden legalny widok nie potrzebuje style, i usunąłem oba atrybuty z listy dozwolonych. Własny filtr CSS to nowa powierzchnia do utrzymywania, żeby pokryć przypadek, który nie istniał.

Ważne w tym znalezisku nie jest rozwiązanie, tylko klasa wektora. Do zaatakowania innego użytkownika nie potrzeba JavaScriptu: CSS-em możesz zasłonić przycisk, przesunąć tekst na inny albo zmienić to, co komunikat pozornie mówi. A każdy widok, który rysuje cudzą treść, nie tylko własną, jest wektorem cross-user.

Sekrety w logach i linki, które żyją za długo

Dwa znaleziska, które nie są błędami logiki, tylko cyklu życia.

Pierwsze: kiedy uwierzytelnianie kluczem API padało z powodu uszkodzonego albo źle sformatowanego klucza, catch wrzucał surowy klucz do kontekstu loga, a logger serializował ten kontekst przez JSON.stringify bez żadnej redakcji. Poprawny klucz wysłany pod zły endpoint lądował w logu czystym tekstem. Redakcja, którą wstawiłem, atakuje z dwóch stron — po nazwie pola (/rawkey|token|authorization|password/i) i po kształcie wartości (prefiks dk_live_ / dk_test_) — i zostawia widocznych osiem pierwszych znaków, a po nich ***REDACTED***, w tym samym formacie, w jakim UI chowa klucz po jednorazowym pokazaniu.

Drugie: podpisane URL-e S3 dla odtwarzacza audio generowały się z expiresIn na siedem dni. Siedem dni to wieczność dla linku, który ma tylko przeżyć załadowanie <audio>. Każdy z linkiem — przesłanym dalej, w historii, zbuforowanym przez firmowe proxy — zachowywał dostęp do oryginalnego audio przez całe to okno, a skasowanie transkrypcji niczego nie unieważniało. Zszedłem z wygaśnięciem do 600 sekund, z odnowieniem po stronie klienta w 480, i dodałem DeleteObjectCommand na obiekcie przy kasowaniu transkrypcji, bo do tej pory kaskada usuwała tylko wiersz.

Koszt w doświadczeniu użytkownika był zerowy poza jednym przepływem: przesłaniem linku do audio mailem koledze. Ten przepływ nigdy nie był wspieraną funkcją ani nie miał żadnej kontroli dostępu i uważam za słuszne, że przestał działać.

Teraz to planuję, zamiast sobie przypominać

Problem z audytami jest taki, że zależą od tego, czy pewnego dnia będzie ci się chciało. Mój wyszedł dlatego, że dokumentowałem decyzje projektowe i przy ponownym czytaniu coś mi zazgrzytało.

Każde znalezisko zamienia się w krótki plik w docs/security/ z wektorem i wyjaśnieniem, dlaczego poprawka jest taka, a nie inna, i z tego powstaje lista, którą przeglądam resztę. Kiedy powtórzyłem ćwiczenie na boilerplacie, od którego zaczynam projekty, wyszło trzydzieści dziewięć punktów między bezpieczeństwem, SEO i architekturą, a większość była wersjami tej samej garstki pytań: co się dzieje przy pustej liście, kto egzekwuje uprawnienie w wewnętrznej pętli, jaki jest sufit tego limitu, jak długo żyje ten link.

Czytanie kodu po to, żeby potwierdzić, że działa, i czytanie go po to, żeby zobaczyć, jak go zepsuć, to dwie różne lektury. Druga nie dzieje się sama przy okazji pierwszej.