• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

bezpieczne logowanie

Object Storage Arubacloud
0 głosów
285 wizyt
pytanie zadane 9 maja 2018 w PHP przez Marchiew Dyskutant (7,690 p.)

Cześć,

Mam hopla a punkcie bezpieczeństwa jednocześnie nie bardzo, tak naprawdę, się na tym znając, co sprawia że wole nie napisać projektu niż żeby był dziurawy... Walczę z tym, bo od półtorej roku stoję w miejscu z umiejętnościami.

Dla testu napisałem najprostszy z możliwych skrypt na zalogowanie się. Czy jest to pójście w dobrą stronę?

if (isset($_POST['nick'])) {
	$nick = $_POST['nick']; 		unset($_POST['nick']);
	$password = $_POST['password']; unset($_POST['password']);
	
	// nick zastepuje email, cos unikatowego i dlatego po nim sprawdzam
	$user = DB::query("SELECT * FROM users WHERE BINARY nick = :nick", [
		DB::bind(':nick', $nick, "/^[a-zA-Z]+$/")
	]);
	
	// wiem że do tego jest metoda z pdo ale nie zaimplementowalem jej u siebie w klase
	$countuser = $user->fetchAll();
	$user = $user->fetch();
	
		
	if (count($countuser) == 1) {
		
		// nie hashowalem hasla bo nie to bylo moim celem
		
		// sposob 1
		
		$validNick = preg_match("/^" . $user['nick'] . "$/", $nick);
		$validPassword = preg_match("/" . $user['password'] . "$/", $password);
		
		if ($validNick && $validPassword) {
			echo $user['note'];
		} else {
			echo "niepoprawne dane mój Panie";
		}
		
		/*
		sposób 2
		
		$validNick = preg_match("/^" . $user['nick'] . "$/", $_POST['nick']);
		
		if ($validNick) {
			$validPassword = preg_match("/" . $user['password'] . "/", $_POST['password']);
			
			if ($validPassword) {
				echo $user['note'];
			} else {
				echo "niepoprawne hasło mój Panie";
			}
		} else {
			echo "nie ma takiego nicku mój Panie";
		}
		*/
	} else {
		echo "cos poszlo nie tak mój Panie";
	}
}

?>

<!DOCTYPE html>
<html>
<head>
	<meta charset="utf-8">
	<title>bezpieczne logowanie</title>
</head>
<body>
	<form method="post" action="<?= $_SERVER['REQUEST_URI']; ?>">
		<label>
			login
			<input type="text" name="nick" />
		</label>
		<label>
			password
			<input type="password" name="password" />
		<label>
		<button>logn in</button>
	</form>
</body>
</html>


<?php

// wkleiłem klase z innego projektu by sie nie babrac z bazą od zera

class DB
{
    private $stmt;
    private static $values;

    public static function query(string $sql, array $cons): \PDOStatement
    {
        try {
            $dbObject = new DB();
            $pdo = $dbObject->connect();

            $dbObject->stmt = $pdo->prepare($sql);
            $pdo = null;
			
			if (count($cons) > 0) {
				if (in_array(false, $cons)) {
					echo "niepowodzenie walidowania danych";
					//exit();
				}

				if (self::$values) {
					foreach (self::$values as $key => $value) {
						$dbObject->stmt->bindValue($key, $value);
					}
				}
			}

            self::$values = null;
            
            $dbObject->stmt->execute();

            return $dbObject->stmt;

        } catch (PDOException $err) {
            echo $err->getMessage();
            exit();
        }
        
    }

    public static function bind(string $alias, $val, string $con): bool
    {
        $defaultBindParam = array(
            "int" => "/^[0-9]+$/",
            "float" => "/^[0-9]+$/",
            "double" => "/^[0-9]+$/",
            "string" => "/^[0-9a-zA-Z]+$/",
            "bool" => "^(1|0)$",
            "date" => "/^[0-9]{1,4}-[0-9]{2}-[0-9]{2}$/"
        );

        if (isset($defaultBindParam[$con])) {
            return DB::preg($alias, $val, $defaultBindParam[$con]);
        } else {
            return DB::preg($alias, $val, $con);
        }
    }

    private function connect(): \PDO
    {
        try {
            return new PDO("mysql:host=localhost;dbname=bezpiecznelogowanie", "root", "", [
                PDO::ATTR_EMULATE_PREPARES => false,
                PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
                PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
            ]);
        } catch (PDOException $err) {
           echo $err->getMessage();
           exit();
        }
    }

    private static function preg(string $a, $v, string $c): bool
    {
        $c = trim($c, "/");

        if (!preg_match("/" . $c . "/", $v)) {
            return false;
        }

        self::$values[$a] = $v;
        return true;
    }
}

 

komentarz 9 maja 2018 przez 0e85dc6eaf Dyskutant (8,840 p.)

Dlaczego tak

$validNick = preg_match("/^" . $user['nick'] . "$/", $nick);
$validPassword = preg_match("/" . $user['password'] . "$/", $password);

?

komentarz 9 maja 2018 przez Marchiew Dyskutant (7,690 p.)
Czemu

zmiennaBool = preg_match(zmienna, warunek)

a nie

preg_match(zmienna, zmienna wynikowa, warunek) ?

czy w ogole czemu uzywam preg_match?

Uwazam preg za bardzo bezpieczne i malo awaryjne rozwiązanie pod kątem poprawnosci danych oraz ma sie przy nim 100% kontrole jaką postać mają przyjmowac dane zewnętrzne.
komentarz 9 maja 2018 przez 0e85dc6eaf Dyskutant (8,840 p.)
Ale przecież chcesz sprawdzić czy login i hasło są identyczne, więc operator `===` będzie tu lepszy
komentarz 9 maja 2018 przez 0e85dc6eaf Dyskutant (8,840 p.)
Poza tym teraz sprawdzasz czy hasło kończy się właściwym hasłem, a nie czy jest identyczne
komentarz 9 maja 2018 przez Marchiew Dyskutant (7,690 p.)
hmmmmmm, no dobra. To co napisalem działa bardzo dobrze, ale mogę poprawić na twoją wersję (czemu nie). Ale mi chodzi o bezpieczeństwo czyli czy ktos bedzie mogl mi cos wstrzyknąć, podejrzeć co chce ONIONEM... i tu się moja wiedza na ten temat właśnie kończy.
komentarz 9 maja 2018 przez Marchiew Dyskutant (7,690 p.)

@SPAP, A nieeee! To jest literówka którą dopiero teraz zauważyłem. Oczywiście że ma być /^hasło$/.

2 odpowiedzi

+4 głosów
odpowiedź 9 maja 2018 przez Ehlert Ekspert (212,670 p.)

Jeśli chcesz żeby było bezpiecznie, to zacznij korzystać z frameworków i dedykowanych pakietów. To i tak nie wszystko, bo trzeba z tego korzystać też mądrze. 

Co do Twojego kodu:

  1. Nie mam pojęcia po co się męczyć regexpami, skoro to co chcesz uzyskać już jest. Bindowanie w PDO, filter_var, respect validation i wiele innych. 
  2. Całkowicie nie wziąłeś pod uwagę CSRF.
  3. Logika użycia słowa class w tym przypadku jest tak znikoma, że lepiej i czytelniej byłoby to napisane proceduralnie.
  4. Rozumiem że przy większych projektach i bardziej skomplikowanej logice dla 50 query łączymy się z bazą 50 razy.
  5. Piszesz o bezpiecznym systemie i... Nie hashowałem, bo nie było to moim celem. Litości xD
  6. Jak znajdziesz więcej niż jednego usera to powinieneś wywalić błąd, exception, wpis do logów. To oznaka krytycznej niespójności danych w bazie. 
komentarz 9 maja 2018 przez Marchiew Dyskutant (7,690 p.)
Dzięki za odpowiedź.

1. Nie mam jeszcze tego wszystkiego w swojej klasie od bazy i dlatego chciałem coś sprawdzonego i lubianego przeze mnie

2. Nie wiem co to jest.

3. Ta klasa jest wzięta z innego projektu. Tak dla wygody.

4. Niee. Wkleiłem wszystko do jednego pliku bo nie chciało mi się dla tak prozaicznego przykładu dzielić na warstwy

5. Chodziło o zabezpieczenie przed dobraniem się do danych w ogóle, a nie zabezpieczeniu samych danych. W prawdziwym projekcie rzecz jasna, że bym to zrobił.

6. Tego nie wiedziałem. Zawsze uznawałem to za rzecz, która nie powinna się zdarzyć i ewentualnie obsłużyć to zwykłym ifem. Dzięki za informacje.
+1 głos
odpowiedź 9 maja 2018 przez 0e85dc6eaf Dyskutant (8,840 p.)

Dobra, to tak. Twoja strona jest podatna na reflected XSS

<?php
$ch =curl_init();
curl_setopt_array($ch,[
	CURLOPT_URL=>'linkdotwojejstrony/?"><script>alert(1)</script>',
	CURLOPT_RETURNTRANSFER=>1
]);
echo curl_exec($ch);

Prawdopodobnie ryzyko jest bardzo małe, że zadziała to w normalnych warunkach, bo przeglądarka zakoduje < > " jako %3e %3c i %22 (dlatego dałem skrypt PHP), ale na pewno lepiej być świadomym takiego problemu.

Poza tym, nie sprawdzasz czy zmienna $_POST['password'] istnieje, przy obecnym kodzie nie jest to problem bezpieczeństwa, bo funkcja preg_match() poradzi sobie z nieistniejącą zmienną (lub tablicą zamiast stringa), ale też mogłoby to stanowić zagrożenie w niektórych przypadkach, więc lepiej to poprawić.

Oprócz tego, tak jak pisałem wcześniej, regex do hasła sprawdza tylko czy hasło kończy się hasłem zapisanym w bazie, a nie czy jest identyczne. Nie wiem też w jaki sposób walidujesz dane przed wstawieniem do bazy, ale wykonywanie wyrażeń regularnych po stronie serwera niekoniecznie jest bezpieczne. Można stworzyć wyrażenie, które pochłonie dużo zasobów serwera (poszukaj sobie w internecie regex dos attack).

komentarz 9 maja 2018 przez Marchiew Dyskutant (7,690 p.)
edycja 10 maja 2018 przez Marchiew
Czasem mam wrażenie, że curl powstał by włamywacze mieli łatwiej...

W wyrażeniu przy haśle przez nieuwage pominąłem „^” i byłem pewny, że jest.

Podobne pytania

0 głosów
3 odpowiedzi 423 wizyt
+1 głos
1 odpowiedź 629 wizyt
+4 głosów
2 odpowiedzi 434 wizyt

92,555 zapytań

141,403 odpowiedzi

319,559 komentarzy

61,940 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Kolejna edycja największej imprezy hakerskiej w Polsce, czyli Mega Sekurak Hacking Party odbędzie się już 20 maja 2024r. Z tej okazji mamy dla Was kod: pasjamshp - jeżeli wpiszecie go w koszyku, to wówczas otrzymacie 40% zniżki na bilet w wersji standard!

Więcej informacji na temat imprezy znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...