Jak mogę zmusić programistów do zaprzestania pisania kodu podatnego na iniekcję SQL?

11

Czasami jesteś zajęty i przekazujesz małe zadania młodszym programistom. Ale jeśli nie zwrócisz wystarczającej uwagi, znajdziesz się przy tego rodzaju kodzie w produkcji:

class DivtoggleController extends Zend_Controller_Action {

    public function closeAction() {
        /* ... code removed for brevity ... */

        $req = $this->getRequest();
        $formData = $req->getPost();

        $d = $formData['div'];
        $i = $formData['id'];

        $dm = new Model_DivtoggleManager();
        $rs = $dm->setDivToggleById($d, $i);

    }

}


class Model_DivtoggleManager extends Zend_Db_Table {

    public function setDivToggleById($div, $id) {
        $result = $this->getAdapter()->query(
           "update div_toggle set " . $div . "=1 where id=" . $id
        );
    }

}

Biorąc pod uwagę, że usunąłem logikę zarządzania uwierzytelnianiem / sesjami dla zwięzłości, kto może mi powiedzieć, jaki może być problem z tą próbką?

Roger Halliburton
źródło
Dlaczego nie jest to przechowywane w prostym pliku cookie?
Darknight
1
@Darknight, zakładasz, że istnieje sesja do przechowywania pliku cookie. To, czy jest, czy nie, nie ma znaczenia dla większego problemu bezpieczeństwa.
Roger Halliburton,

Odpowiedzi:

24

Możesz ich uczyć. Wszyscy robią to na początku, nawet ty. Jeśli ten typ kodu trafia do produkcji, jest to wina starszych osób; nie młodszy.

Edytować:

Jedną z rzeczy, które zrobiłem, jest to, że osobiście zdecydowałem się proaktywnie prosić ludzi o sprawdzenie mojego kodu (w tym juniorów) przed wydaniem. Kod jest sprawdzany, młodzież postrzega go jako doświadczenie edukacyjne, ludzie tracą obawy przed recenzją kodu jako karą i zaczynają robić to samo.

John Kraft
źródło
2
+1 Musisz się zgodzić. Nie możesz winić (przypuszczalnie) młodszego programisty, ponieważ przyjąłeś poziom wiedzy, którego mogą nie posiadać. (To powiedziawszy, którą lubią myśleć kwestie te są dobrze znane w dzisiejszych czasach Potem znowu, będę. Lubię myśleć, że jestem utalentowany i przystojny, etc.) :-)
John Parker
Wystarczająco uczciwe oświadczenie. Chyba powinienem zadać kolejne pytanie, pytając, dlaczego kierownictwo miałoby nalegać na uruchomienie kodu produkcyjnego, który nie został zatwierdzony przez starszych programistów. Czy ryzykuję swoją pracę z powodu drobnych problemów bezpieczeństwa?
Roger Halliburton
1
@Roger Halliburton: Cóż, jeśli to kwestia bezpieczeństwa nie jakoś wykorzystana, co jest najgorsze, co może się zdarzyć? A dlaczego wskazywanie problemów z bezpieczeństwem kosztowało cię pracę? Czy kierownictwo nie chce się tym zająć?
FrustratedWithFormsDesigner
1
@Roger - jest niewielki, dopóki nie zostaniesz zhakowany. :) Myślę, że wprowadzenie kodu do produkcji bez recenzji (bez względu na poziom programisty) jest błędem. Niestety jest to bardzo powszechna praktyka w wielu firmach; i, ime, zwykle zajmuje to znacznie więcej niż $ 4! t, zanim zarząd zacznie doceniać takie rzeczy, jak recenzje kodu.
John Kraft
1
@Roger: Cały kod powinien zostać przejrzany, a nie tylko kod napisany przez „młodszych” programistów. Nawet starsi programiści popełniają błędy.
Dean Harding,
20

Włam ich kod do oczu, a następnie pokaż im, jak to naprawić. W kółko, dopóki nie zrozumieją.

Joe Phillips
źródło
4
Wysyłaj je codziennie: „Nawiasem mówiąc, ostatniej nocy usunąłem twoją bazę danych z powodu innej luki w iniekcji SQL, którą pozostawiłeś w kodzie. Wiem, jak bardzo lubisz przywracać kopie zapasowe bazy danych!: D”
Ant
2
@Ant: Bądź miły. Zrób to krótko po zaplanowanym utworzeniu kopii zapasowej, a nie wcześniej.
Donal Fellows,
@Donal: zrób to krótko po utworzeniu kopii zapasowej, a następnie powiedz im, że tworzenie kopii zapasowej nie powiodło się, i pozwól im się pocić przez resztę dnia, zanim przywróci kopię zapasową na noc.
jwenting
8

Możesz upoważnić ich do wzięcia udziału w zajęciach, gdy tylko dołączą do Twojej firmy, zanim uzyskają dostęp do kontroli źródła, co wprowadza ich w zastrzyki SQL, skryptowanie między witrynami, fałszowanie żądań między witrynami i inne typowe luki w zabezpieczeniach. Omów przykłady twarzą w twarz, przełam zły kod przed nimi, poproś, aby złamali zły kod i skieruj je na stronę OWASP, aby uzyskać więcej informacji, gdy „ukończą” szkołę.

Możesz dodatkowo nakazać korzystanie z niestandardowej biblioteki, która to załatwi, ale to tylko drugorzędne rozwiązanie, ponieważ na pewno będą uruchamiać niestandardowe zapytania, gdy stanie się to wygodniejsze.

Jeśli masz zasoby, przydatne może być również zapewnienie, że więcej starszych członków zespołu zweryfikuje swoje różnice przed zatwierdzeniem.

Wiedza to potęga!

yan
źródło
3
Lepiej wyeliminować ich, zanim dołączą do Twojej firmy. Jeśli zatrudniasz kogoś do napisania czegokolwiek, co korzysta z SQL, nie zadajesz pytań podczas rozmowy kwalifikacyjnej na temat granic zastrzyków w przypadku niekompetencji.
Wooble,
Generalnie się zgodzę, ale bardzo mądry i ambitny wynajem młodszych jest o wiele, znacznie tańszy niż „programista PHP z N-letnim doświadczeniem”, który nie gwarantuje, że będzie o wiele lepszy. Inteligentni młodsi deweloperzy mogą naprawdę szybko zebrać te rzeczy i być dużym atutem.
Yan
3

Zakładając, że jest to niepewność, o której wspominali inni, jako programista dowolnego poziomu, łatwo zapomnieć, że getPost () nie zabezpiecza najpierw danych.

Jednym ze sposobów jest:

  1. Napisz klasę, która pobiera wszystkie dane POST / GET i zapisuje ją taką, jaka jest w klasie Singleton o nazwie „insecure_data”. Następnie wyczyść tablice POST / GET.
  2. Deweloperzy muszą pobierać dane POST / GET z tablicy „insecure_data”, a nie tablice POST / GET.

Każdy programista, który pobiera coś z tablicy o nazwie „insecure_data” i nie zadaje sobie trudu, aby to zabezpieczyć, jest ignorantem lub leniwy. Jeśli jest to pierwsze, zapewnij szkolenie, po którym musi być to drugie - i wtedy masz problem dyscyplinarny, a nie programistyczny.

Dan Blows
źródło
Wow, to jest niepotrzebnie zawiłe i wciąż nie rozwiązuje problemu. Nie chodzi o szorowanie danych wejściowych dla zabawy, ale szorowanie danych, które umieszczasz w bazie danych, a dane te teoretycznie mogą pochodzić z dowolnego miejsca, z formularza internetowego, wiadomości e-mail lub dokumentu XML, który ktoś przesyła. We wszystkich tych przypadkach musisz wyczyścić, gdy trafi do bazy danych.
Roger Halliburton,
@RogerHalliburton Oczywiście nie rozwiązuje wszystkiego, ale jest to koncepcja (rodzaj wzorca projektowego, jeśli chcesz), a nie w pełni rozwinięty środek bezpieczeństwa, który sprawia, że ​​niepewne dane wyglądają niepewnie.
Dan Blows,
O, rozumiem. Ale walczysz tylko z ludzką naturą, jeśli powiesz komuś „Ten obiekt jest niepewny, chyba że zostanie oznaczony jako bezpieczny” i spróbuje go użyć do czegoś, przez większość czasu oflaguje go jako bezpieczny bez faktycznego sprawdzania. Cóż, ciesz się poprawą reputacji.
Roger Halliburton
Z punktu widzenia młodszego programisty samo zobaczenie $ insecure_data podnosi flagę w taki sposób, że samo zobaczenie $ postdata (lub cokolwiek innego) nie. Pomysł pochodzi z Joela Spolsky'ego „Niech zły kod wygląda źle” . Jeśli ludzka natura programistów ignoruje tę czerwoną flagę, musisz zapewnić dużo szkolenia lub pozyskać nowych programistów.
Dan Blows
Nie trzymam Spolsky'ego na piedestale. Są to programiści, którzy łatwo ignorują niespójne tabulacje, nie zamierzają zastanawiać się nad użyciem czegoś, co nazywa się „niepewne”.
Roger Halliburton
1

Jednym z najlepszych przewodników, które przeczytałem na temat bezpieczeństwa w sieci, jest ten przewodnik bezpieczeństwa Ruby on Rails . Chociaż jest to Ruby on Rails, wiele koncepcji ma zastosowanie do każdego projektu internetowego. Zachęcam każdego nowego do przeczytania tego przewodnika.

mistagrooves
źródło
2
Wszyscy wiedzą, że Ruby nie jest bezpieczna.
Roger Halliburton,
5
@Roger: „Wszyscy wiedzą” wszelkiego rodzaju rzeczy, które mogą, ale nie muszą być prawdą. Opowiadam się za podejściem opartym na rzeczywistości, a nie na poglądzie.
Donal Fellows,
0

Kod, który podałeś powyżej, jest podatny na atak wstrzykiwania SQL, ponieważ dane HTTP używane w zapytaniu nie zostały wyczyszczone mysql_real_escape_stringani w żaden inny sposób.

Ryan
źródło
1
och, myślałem, że odpowiadamy na to pytanie testowe:]
Ryan
Przegłosowania są nieco kiepskie, ponieważ brzmienie pytania zostało zmienione ex post: P
Ryan
0

Jeśli chodzi o twoje (przypuszczalnie nadrzędne) pytanie „w jaki sposób mogę zmusić programistów do zaprzestania robienia tego”, powiedziałbym, że regularnie ich mentorujesz, dokładnie wyjaśniając daną kwestię (i potencjalny błąd itp.) I podkreślając znaczenie luk w zabezpieczeniach kodu (zarówno pod względem wstrzykiwania SQL, jak i skryptów krzyżowych itp.) jest prawdopodobnie najbardziej sensownym rozwiązaniem.

Jeśli mimo wszystko powyższego nadal się psują (możesz rzucić okiem na ich zobowiązania, itp. Zamiast odkrywać „na żywo”), problem polega na tym, że ich zawiodłeś jako mentora, lub że być może muszą znaleźć coś bardziej odpowiedniego do życia.

John Parker
źródło