Własny prosty CMS w PHP

Odpowiedz Nowy wątek
2012-07-05 14:17
1

Napisałem prosty skrypt CMS - systemu zarządzania treścią. Niestety okazał się on bardzo dziurawy, więc obecnie jest już wersja 3. Oto link do pobrania:

http://wrzucacz.pl/file/7831341490468

Co o nim sądzicie? Czy jest bezpieczny? Czy wszystko działa poprawnie? Po instalacji dane do PA /admin
Login: admin
Hasło: haslo


Moja strona: materdefense.hostzi.com

Pozostało 580 znaków

2012-07-05 22:00
0

po pierwsze kilka Twoich punktów dla czego warto używać Twojego CMS'a jest trochę naciągana:) ale do konkretów
dlaczego trzymasz hasło w bazie w postaci jawnego tekstu? Zahaszuj go choćby głupią MD5. Najlepiej dla każdego użytkownika dodaj tak zwany salt (sól) i tą solą posól hasła. Wtedy trudniej będzie zgadnąć je jest hasło znają jego hash.

Jeśli chodzi o kategorię to co jeśli... bym chciał mieć ich 6? :P albo 16? zastanów się jakby to rozwiązać bardziej uniwersalnie. I nie będziesz musiał trzymać info o nich w plikach.

Gdy wszedłem do aktualizacji systemu i kazałem sprawdzić, czy jest aktualna to... nic się nie zmieniło :)

to tak na szybko :)

Jak ma MD5 hashować to niech lepiej w ogóle nie hashuje. Ludzie, ten algorytm nie służy do hashowania haseł, nawet nazwa to sugeruje :| - Demonical Monk 2012-07-05 22:11
dlatego napisałem "Zahaszuj go choćby głupią MD5" byleby coś było - no_solution_found 2012-07-05 22:40

Pozostało 580 znaków

2012-07-05 22:40
<?php // Jeśli istnieje pasek boczny dodaj tabelkę
if($pasekboczny_spr == 613) { // Jeśli $pasekboczny_spr jest równy 627 to znaczy że istnieje
    include('include/pasekbocznyspr.php'); // Tabela HTML/PHP z paskiem bocznym
}
else
{
}
?>

Po co pusty blok else? Komentarze albo opisują oczywiste rzeczy, jak echo $naglowek; // wyswietlanie naglowka, albo wręcz bezsensowne i niezrozumiałe jak wyżej.

<html>
<head>

<?php
include('include/baza.php');
?>

Naprawdę nie zalecam osadzania skryptów "na chama" w HTMLu, to strasznie utrudnia dostosowanie szablonu. Poczytaj o systemach szablonów, np. Smarty. Taki system pozwala oddzielić rzeczy związane z wyglądem i prezentacją od reszty kodu.

if(isset($_GET['id']) && !is_array($_GET['id']) && preg_match('/^[0-9a-zA-Z_]{1,25}$/',$_GET['id'])) {
    if(in_array($_GET['id'],$kategorie)) {
        include('kategorie/'.$_GET['id'].'.txt');
    }

Na szczęście całkiem sensowna weryfikacja danych od użytkownika, jesteś jednym z nielicznych newbie, którzy nie otworzyli miliarda naprawdę trywialnych luk bezpieczeństwa :P

<!-- index.php -->
<meta http-equiv="Content-Type" content="text/html; charset=uft8" />

<!-- admin/pasek.php -->
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2" />

Używasz różnych kodowań, to się nie powinno skończyć zbyt dobrze. Zdecyduj się na jedno kodowanie i konsekwentnie wykorzystuj je w każdym miejscu w projekcie.

$login = addslashes($_POST['login']);
$haslo = addslashes($_POST['haslo']);
$ip = addslashes($_SERVER['REMOTE_ADDR']);

mysql_query('INSERT INTO logi SET login=\''.$login.'\', haslo=\''.$haslo.'\', ip=\''.$ip.'\'');

Źle. Użyj biblioteki PDO lub jakiegoś ORMa.
Nie powinno się podstawiać do zapytania danych przepuszczonych przez addslashes (wyjaśnienie).

Generalnie jakieś zadatki widać. Widać, że umiecie wykombinować coś pomimo tego, iż posiadacie dość ograniczoną wiedzę. Rozumienie tego co się robi w jak największym stopniu to jedna z najbardziej pożądanych cech programisty. Papy z dachu nie zrywa, ale jest i prawdopodobnie działa. Jak na początek to i taką aplikację można nazwać osiągnięciem ;] Ćwiczcie dalej.


Women were the reason I became a monk - and, ah, the reason I switched back...
edytowany 3x, ostatnio: Demonical Monk, 2012-07-05 22:44
Pierwszy raz widzę jak kogoś chwalisz :>, to znaczy że ten kod musi być przynajmniej tak dobry jak Coyote. - msm 2012-07-06 02:33
Nie chodzi o to. To są newbie, nie profesjonaliści. Kod jest jaki jest, ale widać w nim potencjał. - Demonical Monk 2012-07-06 15:26

Pozostało 580 znaków

2012-07-06 11:20
0

W kwestii zabezpieczeń to raczej się orientuję. Więc tak:

  • skasowałem te else - nie ma po co kodu zapychać
  • przestawiłem to include
  • co do kodowania, strona główna korzysta z bazy, a baza korzysta z utf-8. PA akurat iso. Niech już tak zostanie
  • Dodałem kodowanie hasła w md5

Co do kategorii to pracuję nad tym. Jeszcze jakieś uwagi? Z góry dziękuję

Edit: Dodałem jeszcze zabezpieczenie przy zmianie hasła - należy podać stare. To dla zabezpieczenia przeciw CSRF. Jeszcze zablokuję w bezpośrednich poleceniach do bazy wszystkie operacje z tablicą logowanie


Moja strona: materdefense.hostzi.com
edytowany 3x, ostatnio: materkamil, 2012-07-23 21:52
Dodałem jeszcze zabezpieczenie przy zmianie hasła - należy podać stare. - a co ktoś ma zrobić jeśli zapomni hasła? - msm 2012-07-06 15:03

Pozostało 580 znaków

2012-07-06 15:28
0
  • Dodałem kodowanie hasła w md5

To nie kodowanie, tylko hashowanie. Nie opłaca się używać MD5, ponieważ nie był on nawet projektowany z zamysłem przechowywania haseł. Zainteresuj się blowfishem, do zabezpieczania haseł potrzebny jest algorytm, który nie był optymalizowany pod względem szybkości, wręcz taki, który ma celowo większą złożoność.


Women were the reason I became a monk - and, ah, the reason I switched back...

Pozostało 580 znaków

2012-07-06 15:40
mater
0

W takim razie zrobię tak, dam md5(sha1($haslo)) - na taki prosty CMS to aż za duże zabezpieczenie.

Co jeszcze proponujecie, gdzie widzicie błędy, co poprawić, co dodać, itp.

Pozostało 580 znaków

2012-07-06 15:42
0

Źle. Podwójne hashowanie spowoduje tylko tyle, że wygenerujesz znacznie więcej kolizji, a poziomu zabezpieczeń nie podniesiesz. Nie olewaj, poczytaj o blowfishu.


Women were the reason I became a monk - and, ah, the reason I switched back...

Pozostało 580 znaków

2012-07-06 16:01
mater
0

Chcę zastosować proste kodowanie, zrobię to tak, aby uniemożliwić atak słownikowy i utrudnić atak brute force:

sha1($haslo.'matercms'.$haslo)

Pozostało 580 znaków

2012-07-06 16:03
0

Ja zrobiłbym:
sha1($data_rejestracji_usera.$hasło.$coś_innego) (nie wiem, jakie dane trzymasz w bazie).


No i to nie jest kodowanie, tylko hashowanie+to, co powiedział @Demonical Monk - lepiej zastosuj blowfish.


edytowany 2x, ostatnio: Patryk27, 2012-07-06 16:04

Pozostało 580 znaków

2012-07-06 16:07
0

A jeżeli dwóch użytkowników będzie miało takie samo hasło? Przy twoim podejściu wygenerujesz taki sam skrót SHA-1 i atakujący będzie wiedział które hasła są stosowane najczęściej i to od nich zacznie łamanie.

Data rejestracji jest dość dobrą solą o ile jest podana w dość dokładnej precyzji, co najmniej z dokładnością do sekund.


"Programs must be written for people to read, and only incidentally for machines to execute." - Abelson & Sussman, SICP, preface to the first edition
"Ci, co najbardziej pragną planować życie społeczne, gdyby im na to pozwolić, staliby się w najwyższym stopniu niebezpieczni i nietolerancyjni wobec planów życiowych innych ludzi. Często, tchnącego dobrocią i oddanego jakiejś sprawie idealistę, dzieli od fanatyka tylko mały krok."
Demokracja jest fajna, dopóki wygrywa twoja ulubiona partia.
edytowany 1x, ostatnio: Wibowit, 2012-07-06 16:09

Pozostało 580 znaków

2012-07-06 16:55
mater
0

W bazie jest tylko jeden user - admin. Zostawię tak jak jest (haslo.matercms.haslo).

Pozostało 580 znaków

Odpowiedz
Liczba odpowiedzi na stronę

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0