Karta tytułowa do „Security Code Review Explained” przedstawiająca panel różnic w kodzie z oryginalną i zmienioną linią oraz trzema oznaczonymi etapami poniżej: PATTERNS (deterministyczne), następnie PASS, następnie REASONING (model).
Guides & Insights

Przegląd kodu pod kątem bezpieczeństwa wyjaśniony: czego recenzent szuka w diffie i dlaczego

Autor

Rowan Sterling

Data publikacji

Najnowsze modele · 20Zobacz wszystkie modele →
Benchmarki: Artificial Analysis · aktualizowane codziennie
Powrót do wszystkich wpisów

Przegląd kodu pod kątem bezpieczeństwa to celowe czytanie kodu źródłowego w konkretnym celu: znalezienia sposobów, w jakie można go zmusić do zrobienia czegoś, czego jego autor nie zamierzał. Ta definicja brzmi oczywista, dopóki nie porówna się jej z tym, co w rzeczywistości robi większość zespołów, czyli z przeglądem funkcjonalnym z listą kontrolną bezpieczeństwa pod ręką. OWASP Secure Code Review Cheat Sheet precyzyjnie wyznacza granicę: przegląd bezpieczeństwa różni się od przeglądu funkcjonalnego tym, że koncentruje się na walidacji danych wejściowych, uwierzytelnianiu, autoryzacji, implementacji kryptografii i wektorach ataków, a nie na poprawności. Diff, który działa perfekcyjnie, nadal może dać atakującemu ścieżkę do systemu, a tylko jeden z tych dwóch przeglądów jej szuka.

To, co czyni ten temat wartym artykułu teraz, a nie w 2020 roku, to fakt, że model językowy potrafi kompetentnie, tanio i w sposób ciągły czytać diff — co zmienia ekonomikę przeglądu, nie zmieniając ani jednego z wymiarów poddawanych przeglądowi. Lista kontrolna jest starsza niż modele i nie drgnęła. Drgnęło to, jak często możesz sobie pozwolić na jej uruchomienie.

Czego recenzent tak naprawdę szuka

Ściągawka OWASP dzieli pracę na sześć obszarów, a tym, co w tej liście jest przydatne, jest to, że każdy obszar jest pytaniem, a nie kategorią. Kategorie zachęcają do mechanicznego odhaczania; pytania przynoszą ustalenia.

Card 'What a reviewer is actually looking for' listing six dimensions - input and injection, authn and session, authorization, cryptography, business logic, configuration and secrets - each with the question it answers about a diff.

Dane wejściowe i wstrzyknięcia. Nie chodzi o to, czy kod zawiera mechanizm sanityzacji; chodzi o to, czy cokolwiek dociera do interpretera — powłoki, SQL, silnika szablonów, ewaluatora wyrażeń — i na której warstwie ten wymóg ma być egzekwowany. Diff, który dodaje zapytanie parametryzowane, jest dobry; diff, który dodaje zapytanie parametryzowane do funkcji, której pozostali wywołujący przekazują surowe ciągi, jest diffem, który przeniósł problem.

Uwierzytelnianie i sesja. Kim jest wywołujący i w jaki sposób kod to ustalił? Powracającym błędem nie jest zepsute sprawdzanie hasła, lecz sesja, która przetrwa zmianę uprawnień — następuje podniesienie uprawnień, token zachowuje stare oświadczenia (claims), a godzinę później uprawnienia nadal tam są.

Autoryzacja. Sprawdzanie na poziomie obiektu, za każdym razem, na każdej trasie. Konkretną rzeczą, której należy szukać w diffie, jest nowa trasa, która nie dziedziczy guarda, jakiego mają jej trasy siostrzane, ponieważ middleware jest rejestrowane osobno dla każdego routera, a trasa dodana pod innym routerem po prostu nie będzie objęta.

Kryptografia. Prymityw, tryb, długość klucza i to, gdzie znajduje się klucz. Dwa pytania pozwalają wychwycić większość rzeczywistych ustaleń: czy każdy, kto może przeczytać tę bazę kodu, może też odczytać klucz, oraz czy to porównanie jest takie, które nie może ujawniać informacji o czasie wykonania.

Logika biznesowa.Czy sekwencja może zostać przestawiona, odtworzona lub sterowana wartościami wybranymi przez klienta. Cena, ilość, rola lub przejście stanu, które przychodzą od klienta i nie są ponownie wyliczane po stronie serwera, stanowią znalezisko niezależnie od sposobu walidacji.

Konfiguracja i sekrety. Co ten diff wprowadza do repozytorium, czego nigdy nie powinno w nim być, oraz czy wartości domyślne są bezpieczne tam, gdzie kod faktycznie działa. Flaga debugowania domyślnie ustawiona na true to ustalenie dotyczące produkcji, a nie samej flagi.

Przegląd bazowy a przegląd różnic

OWASP wyróżnia dwa tryby, a ich mylenie jest częstym źródłem zarówno marnowania wysiłku, jak i fałszywego przekonania o pewności. Przegląd bazowy kompleksowo analizuje bazę kodu i jest właściwym narzędziem w przypadku nowej aplikacji lub dużej wersji, wdrażania starszego systemu, spełnienia wymogu zgodności albo analizy po incydencie, gdy coś już poszło nie tak. Przegląd oparty na różnicach analizuje tylko to, co się zmieniło, i jest właściwym narzędziem do pull requestów, commitów, ukończenia funkcji i ciągłej walidacji.

To rozróżnienie ma znaczenie, ponieważ oba podejścia prowadzą do innego rodzaju pracy. Przegląd bazowy kończy się listą zaległości; przegląd różnic kończy się decyzją o scaleniu. Ciągłe przeprowadzanie pierwszego jest zbyt kosztowne, a przeprowadzanie drugiego na bazie kodu, w której nigdy nie wykonano pierwszego, sprawia, że zespoły nabierają fałszywego poczucia pokrycia: każda zmiana przechodzi przegląd, a istniejąca wcześniej konkatenacja SQL pozostaje nietknięta.

Screenshot of the OWASP Cheat Sheet Series index entry for Secure Code Review, showing the introduction describing manual examination of source code for vulnerabilities automated tools miss, the baseline-versus-diff review split with their use cases, and a table of contents covering methodology, patterns, techniques, checklists and tools.

Techniki i to, co każda z nich wychwytuje

OWASP wymienia cztery techniki przeglądu, które odróżnia to, za jakim dowodem podąża osoba przeprowadzająca przegląd, a nie to, jak bardzo są dokładne. Analiza wzorców w kodziepodąża za znanymi kształtami: niebezpieczne wywołanie funkcji, deserializacja niezaufanych danych wejściowych, brakujący test długości. Jest szybka i jest dokładnie tym, w czym narzędzie deterministyczne sprawdza się dobrze. Analiza przepływu danychśledzi wartość od miejsca, w którym wchodzi, do miejsca, w którym jest używana, co jest jedynym sposobem, by odpowiedzieć, czy sanityzator zastosowany trzy funkcje wcześniej nadal ma tu zastosowanie. Przegląd oparty na zagrożeniachwychodzi od możliwości atakującego i posuwa się naprzód, i właśnie w ten sposób wychwytuje się lukę między dwoma pojedynczo poprawnymi komponentami. Przegląd logiki biznesowejczyta przepływ pracy, a nie pojedyncze linie, pytając, jaki ciąg legalnych operacji prowadzi do nielegalnego stanu.

Tylko w przypadku pierwszego z tych czterech dobrze sprawdza się skaner, a tylko w przypadku pierwszego i drugiego dobrze sprawdza się nieukierunkowane przejście modelu. To nie jest argument przeciwko modelom; to argument za tym, by powiedzieć recenzentowi, której techniki chcesz, ponieważ prompt, który prosi o „problemy z bezpieczeństwem”, niezawodnie wywoła analizę wzorców i na tym się zatrzyma.

Co zmieniła sztuczna inteligencja, a czego nie

Badania nad kodem tworzonym przez AI zaostrzyły wąskie gardło przeglądu, zamiast je stworzyć. Badanie NYU dotyczące sugestii GitHub Copilot, opublikowane na IEEE S&P w 2022 r., wykazało, że około 40% wygenerowanych fragmentów zawierało wzorzec możliwy do wykorzystania w 18 kategoriach CWE. Praca Stanford nad zachowaniami deweloperów, opublikowana na ACM CCS w 2023 r., wykazała skutek o większym znaczeniu: deweloperzy korzystający z asystentów byli bardziej przekonani o bezpieczeństwie swojego kodu, tworząc jednocześnie mierzalnie mniej bezpieczny kod. Większa ilość kodu plus większa pewność siebie to problem dla przeglądu.

Model językowy jako recenzent zmienia jedną zmienną w tym równaniu — przepustowość. Może przeczytać każdy pull request, przy koszcie za token, a nie za stanowisko, i można mu wprost zadać sześć powyższych pytań. Nie zmienia pytań i nie zyskuje dwóch właściwości, które czynią recenzję godną zaufania: determinizmu i możliwości wykonania kodu. Ten sam diff recenzowany dwukrotnie może dawać różne ustalenia, poziomy ważności zmieniają się między uruchomieniami, a recenzent nie może dowieść, że oznaczona ścieżka jest osiągalna.

Screenshot of the OrcaCode Review product page on OrcaRouter headed 'Open engine. Open prompt. Open bill.', describing a pull-request reviewer whose rules and merge gate run on GitHub under an MIT licence and are billed per token, with an example finding about an idempotency key that still varies in time.

OrcaCode Review to produktowa postać tej idei i warto przyjrzeć się jej z bliska, ponieważ wybory projektowe ujawniają tryby awarii. Recenzja, która izoluje przypadek awarii jako konkretny, powtarzalny komentarz — ten klucz wciąż zmienia się w czasie, więc ponowiona próba obciążenia tworzy nowy klucz i może spowodować podwójne obciążenie, oto jednolinijkowa zmiana — wykonuje pracę, którą wykonuje recenzent. Recenzja, która zgłasza „potencjalny problem bezpieczeństwa” bez reprodukcji, uprawia analizę wzorców z gorszą precyzją niż sam dopasowywacz wzorców. A silnik reguł i bramek opublikowany na otwartej licencji składa dorozumiane twierdzenie, które warto sprawdzić: że osąd recenzenta również powinien podlegać recenzji.

Dwupoziomowe rozwiązanie, które się sprawdza

Mając deterministyczny skaner i probabilistycznego recenzenta, rozwiązaniem, które sprawdza się w praktyce, jest przydzielenie każdemu pracy, w której jest naprawdę dobry. Skaner uruchamia się przy każdym pushu: sekundy, bez modelu, bez wariancji, wychwytując sekrety, niebezpieczne wywołania API, niebezpieczną deserializację i zmyślone importy. Recenzent uruchamia się przy każdym pull requeście: minuty, oparty na modelu, wychwytując logikę między wierszami — czy sprawdzenie autoryzacji odbywa się przed pobraniem, czy klucz idempotentności jest powiązany ze stabilną wartością, czy nowa trasa odziedziczyła zabezpieczenie.

Potem warstwa, którą ludzie pomijają: bramka. Znalezisko, które nie blokuje scalenia, jest powiadomieniem, a powiadomienia czyta się w piątki. Bramka musi być wąska i absolutna — krytyczne znaleziska blokują, wszystko inne trafia do komentarzy — a odstępstwo musi być spisane, ponieważ wartość bramki polega nie na tym, że zatrzymuje pierwsze znalezisko, lecz na tym, że każde obejście staje się widoczne.

Przeglądanie zmiany, a nie bazy kodu

Praktyczna dyscyplina przeglądu diffu to krótka sekwencja, a jej wartość polega na tym, że można jej nauczyć w jedno popołudnie. Czytaj diff pod kątem tego, co dodaje do powierzchni ataku, a nie pod kątem tego, co zmienia. Dla każdego nowego punktu wejścia prześledź dane wejściowe o jeden przeskok dalej, niż kończy się diff — podatność zwykle znajduje się w funkcji, którą wywołuje, a której autor nie przeczytał. Dla każdej nowej trasy sprawdź rejestrację guarda. Dla każdej nowej zależności sprawdź, czy istnieje i czy ktoś inny opublikował tę nazwę. Dla każdej nowej wartości konfiguracji zastanów się, jaka jest jej wartość domyślna tam, gdzie faktycznie zostanie wdrożona.

Nic z tego nie wymaga dobrej znajomości języka, dlatego metoda ta skaluje się na recenzentów, którzy nie są specjalistami ds. bezpieczeństwa. Wymaga natomiast, aby recenzent widział diff, a nie cały plik, ponieważ cała metoda opiera się na wiedzy o tym, co jest nowe.

A oto uczciwe ograniczenie: przegląd diffu nie potrafi znaleźć wady, która istniała przed diffem. Jeśli aplikacja nigdy nie miała przeglądu bazowego, najbardziej wartościową dostępną pracą z zakresu bezpieczeństwa nie jest lepszy recenzent — jest nią jedno celowe przejrzenie bazy kodu, w obszarach związanych z pieniędzmi, tożsamością i dostępem do plików, z użyciem powyższych sześciu pytań. Potem każdy kolejny przegląd diffu chroni coś, co zostało już raz obejrzane. Wcześniej każdy przegląd diffu chroni coś nieznanego.