hidden.pl
Dzisiejszy materiał sponsorowany jest przez mojego klienta, w związku z tym postaram podać na tyle mało informacji, żeby nie dogrzebać się do danych klienta (czy twórcy kodu), ale na tyle dużo, żeby przestrzec przed głupią wtopą. Nazwijmy klienta K, a twórcą kodu X. Sytuacja rozwijnęła się następująco:
X stworzy? mini serwis na zamówienie K,
po jakimś czasie współpraca pomiędzy K-X się kończy,
przejęcie innego projektu powiązanego z serwisem trafia do nowej osoby (do mnie).
Chyba powinienem tu wspomnieć słowa mojego promotora, że "kod należy tworzyć tak, żeby nikt inny nie przejął projektu". Zatem o komentarzach zapominamy, a im bardziej skomplikowana budowa tym lepiej... Częściowo się zgadzam, ale tylko ze względu na nabyte lenistwo do tracenia energii na niepotrzebny kod typu: komentarze, dokumentacja czy schamaty UML. Wciąż wierzę, że inni programiści postępują tak, z tych samych pobudek ja.
Zderzenie z rzeczywistością wygląda nastąpująco:
- nieporządane zachowanie się serwisu,
- próba niewnikania w szczegóły budowy aplikacji i edytowanie kilku plików systemowych kończy się niepowodzeniem,
- wsteczna inżynieria,
- zdumienie.
Rozumiem, że kusząca jest opcja zapisywania konfiguracji w bazie - szybko i sprawnie. W ten sposób robi np. wordpress i jest to bardzo ok, ze względu na większe możliwości konfiguracyjne. W przypadku wordpressa mamy taką tabelę:
CREATE TABLE `wp_options` (
`option_id` bigint(20) unsigned NOT NULL auto_increment,
`blog_id` int(11) NOT NULL default '0',
`option_name` varchar(64) NOT NULL default '',
`option_value` longtext NOT NULL,
`autoload` varchar(20) NOT NULL default 'yes',
PRIMARY KEY (`option_id`),
UNIQUE KEY `option_name` (`option_name`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8;
Uważam, że przy małych systemach dedykowanych dla małych przedsiębiorców - taka konfiguracja musi (!!!) znajdować się w odrębrnym pliku - tak jak np w phpLD.
W przypadku bazy danych serwisu, X połączył ustawienia i content w jednej tabeli, która wygl?da mniej więcej tak:
CREATE TABLE IF NOT EXISTS `ustawienia` (
`id` int(11) NOT NULL auto_increment,
`nazwa` varchar(50) NOT NULL default '',
`jezyk` varchar(5) NOT NULL default '',
`tresc` blob,
PRIMARY KEY (`id`)
) ENGINE=MyISAM DEFAULT CHARSET=latin2;
Masakra! Dlaczego? Bo pomimo tego, że teoretycznie aplikacja posiada wbudowaną internacjonalizację, to w rzeczywistości gdyby aplikacja rozrosła się do 50-70 zaindeksowanych podstron i ktoś chciałby w pełni je kontrolować dla kilku ustawień lokalnych, to choćby się zesrał ogniem i mieczem, nie ma najmniejszych szans. Kolejna sprawa, to jak można było ustawić treść jako bloby??? Ciężko było zrobić jedną tabelę więcej? Bloby nie są searchable!!!!
Wreszcie - jesli bawimy się internacjonalizacją to korzystajmy z tego co jest...
"$_GET['jezyk']
mam pytanie do X: nie slyszałeś o zmiennej "locale"? 99,9999999% internautów ma ją ustawioną.... tak, tak wiem, ty zrobisz lepszą.....
....to by tłumaczyło, dlaczego w mini serwisie zmienna $jezyk jest ustawiona na 'PL'... nie mówię, żeby od razu szukać specyfikacji (http://www.w3.org/International/articles/language-tags/), czy nawet zaleceniach W3C o negocjacji języka (http://www.w3.org/International/questions/qa-lang-priorities), ale chyba można spojrzeć chociać do ustawień przeglądarki i zobaczyć, że zmienna odpowiadająca za język zawsze rozpoczyna się "xx-".
Ze względu na rozwój sytuacji nastąpił szybki backup, uruchomienie serwisu w wersji lokalnej i wsteczna inżynieria lokalna. Zauwałyłem, że na stronie pojawiają się tego typu dane:
<link href="file:///C:%5CUsers%5CNAZWA_U?YTKOWNIKA%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml" rel="File-List" />
<link href="file:///C:%5CUsers%5C>NAZWA_U?YTKOWNIKA%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_editdata.mso" rel="Edit-Time-Data" />
To zaskutkowało w:
$ grep 'Users.*AppData' * . -R|wc -l
0
$ wget -r http://localhost/
FINISHED --2010-04-02 20:05:06--
Downloaded: 78 files, 7.1M in 2s (369 MB/s)
$ grep 'Users.*AppData' * localhost/ -R|wc -l
66
Wniosek jeden => klient, tworzył tekst w edytorze typu word, a następnie ^C & ^V & Send...
Ktoś może powiedzieć: "Klient sam jest sobie winien: jakie narz?dzia takie rezultaty". Zdecydowanie nie! Jest to jak najbardziej niedoróbka. Pozwalanie na dodawanie danych, bez skontrolowania ich jest proszeniem się o XSS. Najsłabszym ogniwem systemu jest zawsze użytkownik. I nie czepiam się, że dane odwołują się do danych na lokalnej maszynie, ale skoro takie dane przeszły to przejdą każde. Oczywiście, że umieszczanie w zawartości stron danych odwołujących się do danych na lokalnej maszynie, może zakończyć się tragicznie.
Najprostszy scenariusz:
Wystarczy skazić środowisko, np. przy jakimś zdarzeniu np. wysyłania formularzy, dołączyć kod:
AppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml" rel="File-List" />
był zastępowany kodem:
AppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml" rel="File-List" /><script></script>
Rezultat:
- Ludzie odwiedzający serwis: W zależności od tego co było wewnątrz znaczników script, powinni zauważyć zgłoszenie się jakiegoś cudownego programu chroniącego ich tyłek - np. antywirus, w przypadku braku tego typu oprogramowania mogą zauważyć "0 PLN" na swoich kontach internetowych.
- Nasz klient: utrata wiarygodnoś
- Twórca kodu: w przypadku czarnego scenariusza 2 powyższe punkty, odbiją się conajmniej na cv, a jeśli policja nie spieprzy sprawy i dojdzie przyczyny to prawdopodobnie parą wizyt w ramach świadka.
Rozwiązanie:
Każde pole do wrzucania contentu powinno eliminować znaczniki xml. Można kombinować ze zdarzeniami, po wciśnięciu "Send", a przed wysłaniem formularza, ale bez wątpienia najlepszym rozwiązaniem jest ufanie aplikacji po stronie serwera. Jeśli ona jest skażona, to odpowiada za to administrator a nie programista 
Reasumując:
Treść jest Blobem i nie można jej przeszukiwać, to oznacza podwójnie stracony czas na przywracaniu ładu, ale i brak możliwości raportowania takich kwiatków przez chociażby triggery.
Na zakończenie cytat:
"Be liberal in what you accept, and conservative in what you send."
Jon Postel (06.08.1943 - 16.10.1998)