14/3/2024
Od rana zgłaszacie nam, że na stronach mBanku pojawiają się “dziwne znaki”. Te dziwne znaki to kod źródłowy i fakt, nie powinny się pojawić na produkcji, ale przynajmniej te fragmenty kodu, które widzieliśmy, nie zagrażają bezpośrednio bezpieczeństwu banku czy Waszych środków.
Coś się nie sparsowało, temu mBanku
Po kolei pokażmy to, co nam podesłaliście (imiona podsyłających zmienione). Pierwszy napisał Marek, który po wylogowaniu zobaczył coś takiego:
Potem napisała Basia, która zwróciła uwagę na ten kod:
Następnie Andrzej napisał, że jego ulubiony fragment to:
A Norbert uśmiał się z komentarzy i postawił hipotezę, że kod, który się “wysypał” ma już 8 lat:
Z kolei Krzysiek zauważył, że wpadka musiała chwilę trwać, bo niesparsowany kod złapało nawet Google:
Konkurs
Czy jesteście w stanie znaleź błąd, jaki programiści mBanku popełnili w cytowanych powyżej fragmentach kodu? Jeśli tak, wskazanie z uzadnieniem i poprawną konstrukcją możecie wysyłać do nas e-mailem lub dodać w komentarzu poniżej. Pierwsza osoba, która znajdzie buga, dostanie wejściówkę na nasz wykład “Jak nie dać się zhackować?” (gdzie też jest trochę informacji o mBanku ;).
Tylko bez żartów z PHP, to mogło się zdarzyć także w innym języku.
Weź udział z kolegami w jednym z naszych 8 cyberwykładów. Wiedzę podajemy z humorem i w przystępny dla każdego pracownika sposób. Zdalnie lub u Ciebie w firmie. Kliknij tu i zobacz opisy wykładów!
Każdy powinien zobaczyć te webinary! Praktyczna wiedza i zrozumiały język. 6 topowych tematów — kliknij tutaj i zobacz szczegółowe opisy oraz darmowy webinar.
Według mnie błąd jest w screenie 3/5 gdzie ładujemy (require) pliki z requestu. Na upartego można było by wykonać kod zdalnie.
Nie wolno “ufać” danym z requestu szczególnie przy ładowaniu pliku przez podanie jego nazwy.
Uwaga! Odpowiadam na konkurs:
Użyli PHP.
żeby było ciekawiej to nie używaja zadnego frameworka tylko wlasnej implementacji mvc (pseudo) :D
Bo TAGi trzeba lubieć i umieć zamykać. Nawet w PHP.
Moj wewnętrzny programista PHP płacze widząc takie konstrukcje
Na szybko bym powiedział, że ktoś zrobił jakiś misspelling w konfiguracji serwera Apache/Nginx
Mój wewnętrzny programista płacze jak ktoś, kto pisze na PHP, nazywa siebie programistą.
> Mój wewnętrzny programista płacze jak ktoś, kto pisze na PHP, nazywa siebie programistą.
To przygotuj chusteczki – ja dzisiaj robilem sobie pranie I tez nazywam siebie programista! :p
najlepsze jest to że ludzie tak krytykuja php i nazywaja go “nie jezykiem” bo swoja przygode zazwyczaj zaczeli od tutoriala na yt z jakiegos pythona i jedyne co potrafia to korzystac z bilbiotek innych i maja sie za bogów. A okazuje sie w wiekszosci że ci tacy guru programisci z nich, nie znaja podstaw z zakresu dzialania kompa który tak bardzo “programuja”. Ostatnio mialem beke z takiego nażelowanego gogusia ktory to nie mowil co stworzył a jak przyszło co do czego to nie bardzo wiedzial jaka jest roznica miedzy rejestrami procesora (nie mowiac o podstawie jaka jest miedzy x32 a x64), nie wiedzial co to wyjatki i de facto nie wiedzial czym tak naprawde sa zmienne w pamieci xD. Geniusze, co lizneli tematu i maja sie za guru, wiec pluja na wszystko co uznaja za stare, bo tego po prostu nie znają wiec zeby nie wyszlo ze sie znaja to wola opluwac i być anty :D
P.S pisze to ja Jarząbek :D .. co siedzi w programowaniu od 29 lat .. i zaczynalem od pascala (nie nawidze), pozniej c++ (do tej pory uwielbiam i pisze w nim duuuzo), asm i tona innych po drodze, uwzgledniajac py … ale php poznalem juz dawno i robie sobie w nim, bo lubie machnac jakies apki (w przerwie na kawe ;) .. i zgarnac za to duzą i łatwą kase :D
Paweł, zrobiłeś mi dzień! W 100% się z Tobą zgadzam. Programuję w kilku językach, w tym PHP (tak, nazywam siebie programistą PHP). Od końca lat 90 (sic!) słyszę, że PHP nie warto, że się nie rozwinie, że się kończy, że oldskulowe… Pitu, pitu… Tak, jak mówisz, weźmie 15 bibliotek napisanych przez kogoś innego, coś tam sklei i wielki programista, bo w py albo cpp. A jak ostatnio w jednej firmie (nota bene firmie, ktora produkuje soft w cpp) chcieli pewną aplikację na wlasne potrzeby, to usłyszałem, że nie zrobią, bo trzeba zespół, PMa i ludzi nie będzie odciągał na rok z projektów. No to napisałem w miesiąc w PHP. Ale nie przy kawie, tylko przy kanapce, w przerwach na jedzenie…
Nie jestem znawcą PHP, ale w fragmencie znalezionym przez Andrzeja, sądzę, że jest możliwość nadpisania parametrów w zapytaniu do serwera, dokładniej ‘c’ i ‘m’ i tym sposobem można by sprawdzić, czy i jakie pliki istnieją na serwerze.
Poprawna wersja powinna ograniczać to do jakieś predefiniowanej listy.
błedem tam jest nie tylko złe parsowanie, ale tez przypisywanie bez weryfikacji (parsowanie to nie weryfikacja :D) zmiennych i kierowanie żadania do pliku do ktorego mozna nie powinno sie miec dostepu .. bo jak sobie podam jako kontroller -> admin i metode “index” .. ciekaw jestem co bym uzyskał, zamiast wyciagu z ostatnich zakupu z biedronki :D
macie typo: “wskazanie z uzadnieniem”
a bug? jest ich tu z 20. Używanie $_REQUEST, niesprawdzanie $params[‘c’] issetem, global w stringm(), niepotrzebny taki if $m && count($m) > 1, $ret[] = $agent === ‘MSIE’ ? $ret[1] : 0; to jest błędne, floorwanie na $UAData[1] jesli nie jest numerem, if (file_exists(“app/controllers/$controller.php”) && file_exists(“app/models/$model.php”)) niezgodne z MVC (te same nazwy bo $params[‘c’] do modelu!), zakladanie, że $days to liczba 9-bitowa, wszystkie te exity, strach to czytac
Brakuje ? przy zamknięciu tagu “
Fajnie, że znowu same screeny i niewidomy nic z tego nie rozczyta. Pisałem wielokrotnie, że też Was bezwzrokowi czytają.
Screeny dali pewnie dlatego zeby jakies dzieci do rozwiazania zagadki nie skopiowali do jakiegos edytora albo gpt i prosili o odpowiedz na pytanie :/
Kuchnia, dawno nie klepałem kodu ale na drugim screenie nie podoba mi się to co widzę po otwarciu html. Nigdy się nie spotkałem z tym że od razu tam wskakuje od razu “class=”<?php"
Ale być może przez te kilka lat się coś pozmieniało
nie wnikałem po co wyliczają numer dnia w roku, ale zastanawiam się czy ktoś przewidział lata przestępne i czy to jest problem :)
Na pierwszy rzut oka brakuje mi tutaj po prostu znaków otwarcia i zamknięcia bloku kodu PHP: . Na pierwszy screenie widać, że na końcu mamy tylko ‘>’ i brakuje ‘?’.
Czy jesteście w stanie znaleźĆ błąd, jaki programiści mBanku popełnili w cytowanych powyżej fragmentach kodu?
Brakuje Ć w słowie znaleźć, zalicza się? :)
Brakuje też “s” w:
“wskazanie z uzadnieniem”
Błąd: pisali na PHP.
Nie patrząc do kodu nie sprawdzono input użytkownika
Może mieli wyłączony modul php w apache. XD
Zmienne $c i $m dla controlera i modelu są błędnie “wywoływane”. Warunek nie sprawdzi czy istnieją takie pliki i nie dołączy odpowiednich.
Kod się wysypał bo ktoś to wsadził w html i nie sparsowal…
Zmienne $c i $m dla controlera i modelu są błędnie “wywoływane” (“.$c.”ontroller.php itd.). Warunek nie sprawdzi czy istnieją takie pliki i nie dołączy odpowiednich.
Kod się wysypał do przeglądarki bo ktoś to wsadził w html i nie sparsowal…
poprawna odpowiedź:
‘It’s Not a Bug, It’s a Feature.’
Użyli PHP xD
winne jest php, to nie mogło się zdarzyć w innym języku, bo w innym języku są osobne systemy szablonów, a nie, ze język jest systemem szablonów
W innym języku mogłoby się to wydarzyć. A PHP nie jest systemem szablonów. To jawny problem z ustawieniami apache/nginx lub jakaś niedorajda nie zamknęła taga. Choć w dobrze napisanych systemach takie rzeczy się nie zdarzają.
Przecież w PHP już od bardzo dawna się tak nie pisze. Ten kod ma pewnie z 20 lat i pamięta jeszcze czasy PHP4 XD
“Czy jesteście w stanie znaleź błąd” – tak. Błąd znajduje się w słowie “znaleź”. ;)
“Czy jesteście w stanie znaleź błąd, jaki programiści mBanku popełnili w cytowanych powyżej fragmentach kodu?” – nie. ;)
Bezpośrednie wykorzystanie danych z $_Request
I do not always test my code, but when I do…
It’s in production
I do not always test my code, but when I do…
It’s in production ;)
Wydaje się to ze może występować problem z konfiguracją serwera (np. moduł PHP nie jest aktywny lub źle skonfigurowany .htaccess na serwerze Apache) lub może występować błąd PHP, który powoduje, że serwer nie przetwarza poprawnie kodu PHP i przez to wyświetlany jest jako plain text
O ktoś własny framework pisał… I to zaczął na wersji PHP starszej niż 5.3, a 5.3 wyszła w 2009.
I wymyślał własny autload klas na piechotę…
No pewnie jeszcze czasy 4 pamięta i każdy na piechotę pisał ;)
Na zdjęciu Andrzeja ternary operator (skrót if else) powinien sprawdzać warunek $params[‘c’] == cokolwiek co nas interesuje (to samo dla $params[‘m’]). Jeżeli tam jest string to lepiej sprawdzić jego długość.
Kolejna głupota, że w takiej instytucji nie stosuje się type_safe i innych nowości php. Dodatkowo jeżeli ktoś się upiera na php to istnieje coś takiego jak laravel.
W poście Norberta, pętla jest bez sensu, bo warunek zatrzymujący to $ii < 1 a iteracja zaczyna się od 0 (wykona kod pętli tylko raz).
Czyżby mBank i Pocztex mieli ten sam dział IT ?
Pocztex dzisiaj na produkcji wysłał powiadomienie push w sprawie jakiegoś Krzysztofa, a później kolejny push że testują nową aplikacje i że to tylko testowy push.
Co tak nagle wszyscy dzisiaj na produkcji testują ? Najlepsze, że push w pocztex dostałem na WYLOGOWANEJ aplikacji bo nawet nie pamiętam kiedy ostatnio korzystałem z ich usług a apki jakoś nie odinstalowałem.
przeciez wszyscy wiedzą że najlepiej testuje sie na produkcji :D .. i to w dodatku blisko weekendu .. to najlepsza i najmniej stersująca metoda :D
@Piotr
Bo piątek jest, trzeba się spieszyć
Te komentarze o PHP to by se można było darować. Ktoś kto tak komentuje nie widział nowoczesnych aplikacji pisanych w PHP. Nikt poważny nie pisze już takiego pseudo kodu. W JS też tak nikt dawno nie robi (a robili) , ale ze jest hype na jsa i jego frameworki to nikt żółci nie wylewa.
To jest radość, że ktoś dał im gotową piaskownicę i klocki, co pozwala nazwać się programistami. Tymczasem chodzi o co innego. Co tak stary kod nadający się tylko do zaorania robi na produkcji i dlaczego jest to korporacyjna codzienność.
To, że teraz się tak nie pisze, nie znaczy, że nigdy się nie pisało.
Sam pracuje m.in. przy kodzie, który jest rozwijany od 15 lat. I oczywiście poszczególne fragmenty co jakiś czas są zmieniane i przepisywane od nowa, ale nadal istnieją fragmenty, które (pomijając niezbędne zmiany, żeby nadal działało) pamiętają czasy PHP 4. A klient nie ma do wydania kilku mln, żeby przepisać wszystko na nowo np. na Laravela.
Tu kod wygląda na wczesne wersje PHP 5. Wtedy tak się pisało. Tyle, że takie rzeczy, jak jakieś includy bezpośrednio z $_REQUEST to jedna z pierwszych rzeczy, które powinny być wyłapane przy audycie, nawet takim pół automatycznym, wewnętrznie robionym. A skoro nikt tego nie wyłapał, to obawiam się, na jakim poziomie są zabezpieczenia w mBanku (chociaż te obawy miałem już wcześniej, skoro tak poważna instytucja, jak bank, robi testy na produkcji).
Wydaje się, że błąd jest co najmniej tutaj:
”’ $cookieName = ‘__mpl-rtt’;
$cookie = isset($_COOKIE[$cookieName]) ? $_COOKIE[$cookieName] : null;
$cookie = ”;
”’
bo nawet jeśli ten Cookie jest to i go przypiszemy do zmiennej to w ostatniej linii jest zastępowany pustym stringiem.
Proponuję zmianę na coś takiego:
”’ $cookieName = ‘__mpl-rtt’;
$cookie = isset($_COOKIE[$cookieName]) ? $_COOKIE[$cookieName] : ”;
”’
Ehh
Nie wnikam w czym błąd, bo nie klepałem dawno. Jednak śmieszy mnie to, że ludzie krytykują użycie PHP. Zgadzam się, że są inne, młodsze języki. Kto jednak w PHP klepał to zna nie tylko programowanie, ale też unixy i sieci. Jak dziś sypie się kod gdzieś, to jak to jest Java,. net, js (w rozumieniu react, node.js) i nie do końca wiadomo czy to kod, load balancer, firewall i inne, to jak się takich specy za >= 200 netto/h posadzi z równie wybitnymi adminami (po rocznikach 95 to dziś już DevOps hehe) to pikaczu face i każdy mówi że u mnie działa. Wrzucasz PHP który pisal i forki i otwierał socety w kodzie to ISO/OSI od drugiej do siódmej warstwy sprawdzi i znajdzie problem.
…smutna prawda, w którą stronę to idzie :(
@Manwe
+100
Błętem było ich BHP bo się k***y bawią pehapem.
Błąd konfiguracji serwera/.htaccess/php – pliki php są wyświetlane jako tekst, zamiast interpretowane.
Dlaczego kasujecie moje komentarze?
Napisałem wcześniej, że to błąd konfiguracji serwera web / serwera php / .htaccess, ponieważ kod php jest wyświetlany, zamiast być interpretowany i wykonywany.
To nie błąd php, a konfiguracji cgi/fpm. Serwer www potraktował pliki z rozszerzaniem php jako pliki tekstowe/html
Ktoś miał mega ubaw, kiedy wymyślił, że spędzi cały listopadowy dzień 2016 roku programując i debugując przesunięcia binarne w PHP po to, aby umieścić kretyńską liczbę dni w ciasteczku xD
To musiał być albo stażysta, albo senior developer za 120 zł/h. Nie ma innej opcji.
Te błędy mogą mieć konsekwencje – w mediach pojawiają się informacje, że ludziom podobno zginęły pieniądze z kont.
[…] 14 marca zgłaszano błędy na stronie mBanku, o czym informuje Niebezpiecznik.pl: SZCZEGÓŁY TUTAJ […]
Ja wczoraj próbowałem zalogować się na konto żony w mBanku, ale zawołało potwierdzenie z aplikacji. Anulowałem operację, zalogowałem się jeszcze raz i wpuściło mnie bez potwierdzenia. Później już nie udało mi się tego powtórzyć, ale ewidentnie coś nie zadziałało jak powinno.
Widać, że wykryło, że logujesz się na cudze konto. Nie ładnie! A dla mBanku plus.
Potwierdzam , 18.03 coś się działo z logowaniem.
Nigdy nie dodaje przeglądarek do zaufanych + logowałem się w karcie prywatnej i wpuściło mnie bez zatwierdzenia logowania w aplikacji. Adres strony, certyfikat, wszystko się zgadzało. Po zalogowaniu normalnie widziałem swoje konto itd, ale od razu się wylogowałem.
Przy ponownym logowaniu już musiałem potwierdzić logowanie w aplikacji i było info, że to pierwsza operacja z tego dnia, gdzie było to drugie logowanie i powinna to być operacja nr 2.