Hallo,

ik ben bezig met een registratie script voor m'n website. Ik heb me voorgenomen de beveiliging zo goed als ik kan te maken vanaf het begin.
Is dit script veilig? En waarom geeft mysql_real_escape_string niets terug?


<?php
if($_SERVER['REQUEST_METHOD'] == 'POST') {
    $fname = @$_POST['fname'];
    $lname = @$_POST['lname'];
    $uname = @$_POST['uname'];
    $email = @$_POST['email'];
    $password = @$_POST['password'];
    $checkbox = @$_POST['checkbox'];


    $errors = array();

    if(trim($fname)=='') {
        $errors[] = 'U moet uw voornaam invullen';
    }

    if(trim($lname)=='') {
        $errors[] = 'U moet uw achternaam invullen';
    }

    if(trim($uname)=='') {
        $errors[] = 'U moet uw gebruikersnaam invullen';
    }

    if(trim($email)=='') {
        $errors[] = 'U moet uw email invullen';
    }
    
    if (filter_var($email, FILTER_VALIDATE_EMAIL)) {
        
    } else {
        $errors[] = 'U moet een geldig emailadres invullen';
    }

    if(trim($password)=='') {
        $errors[] = 'U moet uw wachtwoord invullen';
    } else {
        $password = password_hash($password, PASSWORD_DEFAULT);
    }
    
    if(!isset($checkbox)) {
        $errors[] = 'U moet de richtlijnen accepteren';
    }
    
    if ($errors==true) {
        $err = true;
    } else {
        $sql = "INSERT INTO users (
        id, fname, lname, uname, email, password
        ) VALUES (
        '', '" . mysqli_real_escape_string($connection, $fname) . "', '" . mysqli_real_escape_string($connection, $lname) . "', '" . mysqli_real_escape_string($connection, $uname) . "', '" . mysqli_real_escape_string($connection, $email) . "', '" . mysqli_real_escape_string($connection, $password) . "'
        )";
        if(mysqli_query($connection, $sql)) {
            echo 'Succes!';
        } else {
            echo "Error: " . $sql . "<br>" . mysqli_error($connection);
        }
    }
}
?>


Alvast bedankt!!!

Mvg
Bedankt eerst welke karakters te wilt toestaan. Wel of geen spaties, en mogen er ook cijfers?

Een voorbeeld is:
"[A-Za-z_0-9]"

Met preg_match() kan je controleren of een waarde aan deze voorwaarde voldoet.
Johan K op 04/09/2015 18:48:46
Trimmen is niet slim op een wachtwoord veld. Het is een extra karakter die mensen kunnen gebruiken in hun wachtwoord.

Ik zeg nergens dat dat de enige controle zou moeten zijn of dat je dit altijd en overal blind zou moeten toepassen. Je moet per veld bepalen wat nodig is. Als een veld verplicht is, is het controleren of deze "leeg" is een minimale controle. Ik zeg dat je de getrimde variant moet vergelijken met een lege string, NIET dat je de invoer moet AANPASSEN naar de getrimde variant.

Johan K op 04/09/2015 18:48:46
Een simpele regex is voldoende

Hoe is een regex geen whitelist?

Johan K op 04/09/2015 18:48:46
Als er een classe word gemaakt zou het "escapen" eigenlijk automatisch moeten gaan.

Nee, niet automatisch nee. Tenzij je een prepared statements laag aan het bouwen bent, in welk geval je beter van PDO gebruik kunt maken (die van MySQLi is brak). Naar mijn mening moet je dit soort dingen niet automatiseren omdat er dan geen bewustzijn meer is.

Daarnaast wil je soms nog steeds de keuze hebben om op bepaalde plaatsen rauwe data te gebruiken, je wilt dan niet hebben dat deze data automatisch ge-escaped wordt. Je wilt niet dat je klasse je op voorhand beperkt in je handelen.

@Aar, je wilt de hele string controleren dit doe je met ^...expressie...$. Daarnaast heb je de de /i modifier die een case-insensitive match doet. Hierbij moet je ook niet vergeten dat $ ook één newline karakter (\n) accepteert wat vaak vergeten wordt en dat de return-waarde van preg_match gelijk moet zijn aan 1.

Een match voor een (niet-lege) string die enkel uit alfanumerieke karakters zou bestaan wordt dus zoiets:

<?php
$input = 'lala123';
if (preg_match('#^[a-z0-9]+$#i', $input) == 1) {
    echo 'input bestaat enkel uit alfanumerieke karakters';
    // en voor de goede orde zou je $input hier nog moeten trimmen!
} else {
    echo 'input bestaat NIET enkel uit alfanumerieke karakters of is leeg';
}
?>

Zelf ook ff testen natuurlijk...

Met accolades zou je nog een minimale/maximale lengte aan kunnen geven.
{exacte_lengte}
{minimale_lengte,}
{mininmale_lengte, maximale_lengte}
Ik gebruik nu:

<?php
    if (preg_match('#^[a-z0-9]+#i', $fname) != 1) {
        $errors[] = 'Fout!';
    }
?>

Maar dit staat wel toe dat ik bv. Jan-" ingeef.
Ah, $ vergeten, zie bovenstaande edit.

Je had dit ook met enige moeite al af kunnen leiden uit mijn vorige post:

Hele string matchen is ^...expressie...$
Thomas van den Heuvel op 04/09/2015 19:45:32
Ik zeg nergens dat dat de enige controle zou moeten zijn of dat je dit altijd en overal blind zou moeten toepassen. Je moet per veld bepalen wat nodig is. Als een veld verplicht is, is het controleren of deze "leeg" is een minimale controle. Ik zeg dat je de getrimde variant moet vergelijken met een lege string, NIET dat je de invoer moet AANPASSEN naar de getrimde variant.

Ik vergelijk niets met een lege string, hou het gewoon lekker simpel en voer direct de regex uit op velden waar nodig is zoals een username & password. Op andere velden zoals checkbox of optionele velden voldoet empty().

Thomas van den Heuvel op 04/09/2015 19:45:32
Hoe is een regex geen whitelist?

Is het ook, maar je maakt het moeilijker dan dat het is.

Thomas van den Heuvel op 04/09/2015 19:45:32
Nee, niet automatisch nee. Tenzij je een prepared statements laag aan het bouwen bent, in welk geval je beter van PDO gebruik kunt maken (die van MySQLi is brak). Naar mijn mening moet je dit soort dingen niet automatiseren omdat er dan geen bewustzijn meer is.

Daarnaast wil je soms nog steeds de keuze hebben om op bepaalde plaatsen rauwe data te gebruiken, je wilt dan niet hebben dat deze data automatisch ge-escaped wordt. Je wilt niet dat je klasse je op voorhand beperkt in je handelen.

PDO bindParam / bindValue escaped alles als string tenzij anders is doorgegeven of je wilt je zelf open stellen om user input direct in je sql te gebruiken.


Voor mijn username validatie gebruik ik deze:

function validateUsername($name){
  return preg_match('/^[A-z0-9_-]{3,16}$/', $name);
}
 echo validateUsername('FooBar_09') ? 'true' : 'false'; // true;


Dit houd in dat alfabetische, numerieke en _ en - karakters mogen, de rest niet.
<?php
function validateUsername($name){
return filter_var($name, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[A-z0-9_-]{3,16}$/")));
}
?>

Nadeel hieraan is dat je de gebruiker niet laat weten, als ie:

- zijn naam te kort heeft ingevuld
- zijn naam te lang heeft ingevuld
- zijn naam illegale karakters bevat
Johan K op 05/09/2015 11:18:56
Ik vergelijk niets met een lege string, hou het gewoon lekker simpel en voer direct de regex uit op velden waar nodig is zoals een username & password. Op andere velden zoals checkbox of optionele velden voldoet empty().

Ik heb het ook niet over wat jij doet, ik voelde mij genoodzaakt op opnieuw/beter toe te lichten wat ik bedoelde, omdat het antwoord wat jij gaf mij deed denken dat ik invoer aan het aanpassen (trimmen) was...

Johan K op 05/09/2015 11:18:56
Is het ook, maar je maakt het moeilijker dan dat het is.

Daarom zeg ik ook dat je per veld moet bepalen wat nodig is...

Johan K op 05/09/2015 11:18:56
PDO bindParam / bindValue escaped alles als string tenzij anders is doorgegeven

Oh really, zet querylogging eens aan en kijk wat PDO daadwerkelijk doet. 9 van de 10 keer negeert deze gewoon typehints en zet om alle DATA quotes heen (in de uiteindelijke query). But hey, don't take my word for it.

Johan K op 05/09/2015 11:18:56
of je wilt je zelf open stellen om user input direct in je sql te gebruiken.

Dat lijkt mij duidelijk niet de bedoeling. Maar goed, het kan zijn dat jij nooit rauwe SQL passages nodig hebt gehad in je query en hier dus nog nooit tegenaan bent gelopen.

Johan K op 05/09/2015 11:18:56
Voor mijn username validatie gebruik ik deze:

function validateUsername($name){
  return preg_match('/^[A-z0-9_-]{3,16}$/', $name);
}
 echo validateUsername('FooBar_09') ? 'true' : 'false'; // true;


Dit houd in dat alfabetische, numerieke en _ en - karakters mogen, de rest niet.


Lees de handleiding van preg_match() eens, hier staat:
preg_match() returns 1 if the pattern matches given subject, 0 if it does not, or FALSE if an error occurred.

Als je je return-value van je functie als boolean wilt behandelen/gebruiken, vergelijk dan met het cijfer 1.

Daarnaast accepteert jouw functie ook de volgende waarde:
"1234567890123456\n"

Yup. 17 karakters. En een linebreak. De volgende keer dat jij een csv-export van je users maakt dan breekt deze misschien wel.


Lukraak reageren op wat iemand zegt is makkelijk.

Proberen te begrijpen en te doorgronden wat iemand bedoelt is aanzienlijk moeilijker.
>> Een match voor een (niet-lege) string die enkel uit alfanumerieke karakters zou bestaan wordt dus zoiets:

Waarom niet gewoon zo?

<?php
$input = 'abc123';
if (ctype_alnum($input)) {
echo 'oke';
} else {
echo 'niet oke of leeg';
}
Ik heb het script geüpdatet. Ik denk dat ik zowat alle tips heb gebruikt...
Dat van die whitelist ga ik niet meer doen omdat ik het nut er niet meer kan inzien. Dank voor het uitleggen van de Whitelist ik zal dit zeker meenemen.
Is het script veilig? Zijn er eventueel nog dingen die beter moeten?
Even tussendoor: weet je hoe vervelend het is als je een wachtwoord intypt op je telefoon en dat die dan automatisch een spatie erachter zet en daardoor dus niet geldig is ?

Een rtrim($iets) zou echt handig zijn.
Ook bij emailadressen: mijn emailadres staat in mijn woordenboek (Swiftkey), dus als ik maar 4 letters type, dan vult hij het aan. Met een spatie. Nu moet ik die altijd verwijderen (heel vervelend) omdat het anders verkeerd kan gaan.
Trouwens: facebook bijvoorbeeld niet. Als je daar je emailadres invult bij het inloggen MET een spatie, dan haalt ie die spatie weg.
Waarschijnlijk kijkt hij eerst of je rauwe input bestaat. Zo niet: is een rtrim() anders dan de rauwe input? Zo ja: probeer het eens met rtrim() erover..

Reageren