[skrypt/www] Ocena skryptu własnego CMS Opcje

[materkamil]

Ostatnio po ocenie mojej stronki okazało się że ma ona dużo luk i błędów. Jednak teraz napisałem coś podobnego, całkowicie samemu, wszystko w PHP. Jest to tak jakby CMS - własny prosty system zarządzania treścią nazwany po prostu od nicku materCMS. Oto link do niego:

http://www.materdefense.hostzi.com/matercms/

Możliwe że jest w nim jakaś luka ponieważ używany jest w nim komunikacja z bazą SQL, includowanie plików, id, do tego jeszcze pracuje on również na plikach tekstowych.
Panel Admina również znajduje się:

http://www.materdefense.hostzi.com/matercms/admin

Myślę że jest on dobrze zabezpieczony i nie powinien mieć luk ale no sami wiecie. Skrypt jest oparty na sesjach. Po ostatnim "incydencie" sam już nie wiem.

Nie chodzi mi o komentowanie wyglądu, bo jego zmienię jeszcze sto razy tylko głównie mechanizmu i tego całego PHP

Kod skryptu jest tu: (zip, 12 KB)
http://wrzucacz.pl/file/9911339606258

Ten post edytował materkamil 13.06.2012, 17:52:01

[Korab]

To jeszcze podaj proszę dane dostępu do panelu admina.

Przechowywanie danych dostępowych do bazy w głównym katalogu w niezabezpieczonym pliku tekstowym bez żadnego szyfrowania nie wydaje mi się dobrym pomysłem.

Ten post edytował Korab 13.06.2012, 17:48:20

[materkamil]

Chwilę poczekamy z danymi, może nie będą dla was - haxiorów potrzebne. Za chwilę oczywiście dam, tylko zabiorę funkcję zmieniania danych SQL (hasło itp).
Już przyznam się że zaliczyłem jedną wpadkę, bo po skasowaniu ciasteczek PA był dostępny bez hasła, oczywiście teraz już zabezpieczyłem to.

Edit: Przed sekundą znalazłem błąd pozwalający na SQL Injection w PA. Zabezpieczyłem i dodałem na wrzucacz nową wersję. Jak ktoś nie chce pobierać to w katalogu admin pliku passupdate.php dodałem htmlspecialchars

Edit: Co do txt - akurat mój serwer nie podgląda plików txt. A co tu można w zamian ciekawego wymyślić?

Ten post edytował materkamil 13.06.2012, 18:19:00

[!*!]

Niczego się nie nauczyłeś podczas analizy poprzednich kodów. Nadal powielasz te same złe nawyki sprzed kilku lat.

Kod

<?

[PHP] pobierz, plaintext 
include("konfiguracja_bazy.php");
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
$q = "select admin_login,admin_pass from admin_pass where admin_id=1";
$zapytanie = mysql_query($q);
[PHP] pobierz, plaintext 

Polecenia SQL piszemy z dużej. Przejdź na PDO.

[PHP] pobierz, plaintext 
$o = fopen("strony.txt","r");
$t = fread($o,filesize("strony.txt"));
fclose($o);
$array = explode("|",$t);
[PHP] pobierz, plaintext 

Dla jednego pliku taka armata?

[PHP] pobierz, plaintext 
if($_GET["id"] == strip_tags($array[0]) || strip_tags($_GET["id"]) == strip_tags($array[1]) || strip_tags($_GET["id"]) == strip_tags($array[2]) || strip_tags($_GET["id"]) == strip_tags($array[3]) || $_GET["id"] == strip_tags($array[4]) || $_GET["id"] == "newsy") {
	include("strony/".$_GET["id"].".txt");
	}
[PHP] pobierz, plaintext 

Tu już pojechałeś po całości Więcej strip_tags() nie mogłeś? i odwołanie do pliku z linku? Kiepski pomysł, patrz niżej.

I tak jak wspomniał przedmówca... Odwołujesz się do czystego tekstu, i to pełni rolę pliku konfiguracyjnego do bazy?
Klik

[materkamil]

Jeśli chodzi o strip tags to widzę że nie zrozumiałeś kodu. Dodając w PA strony od razu dodaje się również hiperłącze. Jako że w id jest sama nazwa usuwamy to hiperłącze. Ten strip_tags jest użyty wcale nie do obrony, lecz do skasowania tego hiperłącza.

Co daje wg. ciebie <?php a <?

Co do armaty, lubię pisać bardzo zrozumiale. Wiem że można to w 1 linijce ale dla mnie tak jest wygodniej.

Co do klika, przecież to jest tylko pokazywanie nieistniejącej strony jako zawartość geta, do tego z strip_tags i nie stanowi to żadnego zagrożenia (chodzi o pozdrowienia dla php.pl)

Do tego z tym plikiem txt. Jak zmienili byście to wy? Bo ja coś pomysłu nie mam.

Ten post edytował materkamil 13.06.2012, 18:46:23

[!*!]

Jeśli chodzi o strip tags to widzę że nie zrozumiałeś kodu. Dodając w PA strony od razu dodaje się również hiperłącze. Jako że w id jest sama nazwa usuwamy to hiperłącze. Ten strip_tags jest użyty wcale nie do obrony, lecz do skasowania tego hiperłącza.

I twoim zdaniem jest to cacy?

Co daje wg. ciebie <?php a <?

Chociażby poprawne kolorowanie skladni w edytorach, apropo, masz źle zakodowane pliki, UTF8 powinno być.
A w include masz coś innego w nazwie pliku niż tekst? Jak nie, to używaj apostrofów, tak samo w echo.

Co do klika, przecież to jest tylko pokazywanie nieistniejącej strony i nie stanowi to żadnego zagrożenia (chodzi o pozdrowienia dla php.pl)

Pewien żeś? Pomyśl, po co mam wstawiać na Twoją stronę cokolwiek...

Do tego z tym plikiem txt. Jak zmienili byście to wy? Bo ja coś pomysłu nie mam.

Stałe. To Ci wystarczy na początek.

[materkamil]

Pewien żeś? Pomyśl, po co mam wstawiać na Twoją stronę cokolwiek...

Ja jestem na 100% pewien. Przecież tagi są wycięte itp. więc można tu wstawić jedynie tekst. Możesz pokazać coś przykładowo w tym co niby błąd stanowi?

A w include masz coś innego w nazwie pliku niż tekst? Jak nie, to używaj apostrofów, tak samo w echo.

Nie rozumię o co chodzi. Możesz powiedzieć troszkę jaśniej

I twoim zdaniem jest to cacy?

No na pewno działa ładnie i wszystko jest ok. Więc chyba jest "cacy"

[!*!]

Patrz tu
Jak wyślę takiego linka Twojemu klientowi, bo mam nowe zlecenie w kieszeni.

Nie rozumię o co chodzi. Możesz powiedzieć troszkę jaśniej

[PHP] pobierz, plaintext 
include 'plik.php';
echo 'cokolwiek';
[PHP] pobierz, plaintext 

Taki kod jest wydajniejszy. Niż:

[PHP] pobierz, plaintext 
include "plik.php";
echo "cokolwiek";
[PHP] pobierz, plaintext 

[materkamil]

No ok, racja z odstraszaniem klientów ale coś poza tym?

Czyli ogólnie oprócz małych błędów nie wpływających na bezpieczeństwo skryptu wszystko jest ok (oprócz tego txt z którym dalej chciałbym coś zrobić)?



Ten post edytował materkamil 13.06.2012, 19:18:26

[!*!]

Jak wrzucę takie linki do spambotów... , Nawet nie wysyłasz żadnych nagłówków że taka strona nie istnieje. Nie walidujesz w żaden sposób danych, nie sprawdzasz ich typów, w ogóle niczego z nimi nie robisz, skrypt jest dziurawy jak sito.

[materkamil]

Jak sito to raczej nie jest bo nie da się dzięki temu przejąć kontroli nad stronką. Jednak na wszelki wypadek zmienię tą opcję i będę wyświetlał zwykły błąd

[!*!]

Jak sito to raczej nie jest bo nie da się dzięki temu przejąć kontroli nad stronką. Jednak na wszelki wypadek zmienię tą opcję i będę wyświetlał zwykły błąd

Skąd wiesz że nie? Myślisz że po co wymyślono walidację danych?

[materkamil]

Walidację stosuje się w określonych przypadkach. Jeśli wszystko jest wycinane a ty wyświetlasz tekst który nigdzie się nie zapisuje to jest to bezpieczne. A wyszukiwarki? Też wyświetlają: Nie znaleziono costam

[Niktoś]

Ograniczaj długość pól input w formularzach- nie lekceważ tego, gdyż ktoś Ci może puścić z dymem stronę(atak na tablice post/get),serwer(DDOS),baze danych(przekroczenie maksymalnej dozwolonej liczby znaków w danej kolumnie,nawet jeśli jest ustawiona na max) .
Wszystkie te możliwe ataki mogą być tragiczne w skutkach, a wszystko przez to,że nie ograniczasz ilości znaków w polu input.
Maksymalne żądanie post to(o ile się nie mylę) jest standardowo ustawione na 8M-przekroczenie tej wielkości może wygenerować błąd i wyciek cennych informacji.
Na bazie danych max varchar wynosi 65535 bytes -przekroczenie tej ilości znaków(w przełożeniu na bity) np.w zapytaniu select może wygenerować error i wyciek informacji(nazwy tabeli/kolumn może nawet i nazwy bazy danych).

Walidację stosuje się w określonych przypadkach.

Walidacje należy stosować zawsze i wszędzie, gdzie użytkownik ma jakiś wpływ na stan aplikacji(najlepiej po stronie serwera- opcjonalnie po stronie serwera i klienta(walidacja typu client-side).

Ten post edytował Niktoś 13.06.2012, 20:35:08

[materkamil]

Ograniczaj długość pól input w formularzach

Takich herezji dawno nie słyszałem

Przecież długość pola input nie ma większego znaczenia dla osoby znającej chociaż podstawy HTML. Wystarczy z własnego serwera utworzyć action do pliku z własnego formularza.

A w tym przypadku user nie ma nic do stanu aplikacji przecież. Czy ja się mylę?

[Niktoś]

Czy ja się mylę?

Mylisz się i to bardzo.Nie ograniczając pól input -możesz władować w nie dozwoloną ilość znaków(treść naprawdę długą) w rezultacie czego żądanie post może nie obsłużyć tego, a tym bardziej baza danych jeśli pójdzie do niej treść z inputa.
Pamiętaj ,że jeden znak w utf-8 wyciśnięty z klawiatury to 8bitów.

Ten post edytował Niktoś 13.06.2012, 20:41:49

[materkamil]

Podaj przykład kodu ograniczonego inputa proszę

[Niktoś]

No nie-atrybut maxlenght .Poćwicz trochę html.

[materkamil]

Tak, spoko właśnie chciałem to usłyszeć. Jeszcze się upewniłem że mówisz głupoty. No to teraz powiedz mi jak ja zrobię coś takiego na localhost:

<form action=http://www.strona.pl/skrypt.php method=post>
<input type=text name=pole>
<input type=submit>
</form>

I wysyłam dane z własnego komputera więc ograniczenie maxlenght mam gdzieś bo POST nie jest nigdzie ograniczona

Ten post edytował materkamil 13.06.2012, 20:51:03

[Niktoś]

No to po prostu twój serwer nie obsłuży żądania jeśli żądanie będzie większe niż wartość stanadardowo ustawiona w php.ini ,czyli 8M, co za różnica gdzie post będzie szedł.Problem doskonale widać przy wysyłaniu dużych plików,gdzie żądanie POSt nie jest w stanie tego obsłużyć.
Nie ograniczenie pól input stwarza możliwość zainicjowania takiego żądania.

Jeszcze się upewniłem że mówisz głupoty. No to teraz powiedz mi jak ja zrobię coś takiego na localhost:

No to tkwij dalej w swej mądrości.Co za tupet-człowiek chce doradzić, no ale jak ktoś pozjadał wszelakie rozumy to nic nie poradzę.Rób jak chcesz. Zapewne nie słyszałeś o POST OVERLOADING, to zapewne kiedyś usłyszysz.

Ten post edytował Niktoś 13.06.2012, 21:07:16