Hallo,

Ik wou jullie even mijn PHP script laten controleren (als jullie tijd hebben). Ik heb de laatste tijd veel doorgenomen bij w3schools.com en andere php informatie websites. Zouden jullie even willen kijken of mijn script veilig is? Kwa SQL-injections etc. (bruteforce protection zit erop :-), en al uitgetest)


<?php
if (date("d-m-Y") <= "26-03-".date("Y"))
{
  $_SERVER['unix'] = strtotime("UTC");
}

if (date("d-m-Y") >= "26-03-".date("Y"))
{
    $_SERVER['unix'] = strtotime("UTC +1 hour");
}

function loggedIn()
{
  if (!empty($_SESSION['id']) && is_numeric($_SESSION['id']))
  {
    return true;
  }
  else
  {
    return false;
  }
}

function getIpadress()
{
  if (!empty($_SERVER['HTTP_CLIENT_IP']))
  {
    $_SERVER['ip'] = $_SERVER['HTTP_CLIENT_IP'];
  }
  elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR']))
  {
    $_SERVER['ip'] = $_SERVER['HTTP_X_FORWARDED_FOR'];
  }
  else
  {
    $_SERVER['ip'] = $_SERVER['REMOTE_ADDR'];
  }
}

function checkWrongAttempts() {
  include($_SERVER['DOCUMENT_ROOT'].'/paneel/includes/init.php');

  getIpadress();
  $ip = sha1($_SERVER['ip']);
  $datum = $_SERVER['unix'];

  $searchForWrongAttempts = $link->query("SELECT * FROM `paneel_foutepogingen` WHERE `ip`='$ip' ORDER BY `datum_foutepoging` DESC");
  if (!$searchForWrongAttempts)
  {
    echo '0+Er is een fout opgetreden, kom later terug';
    exit();
  }
  else
  {
    if ($searchForWrongAttempts->num_rows > 3)
    {
      $wrongAttempts = $searchForWrongAttempts->fetch_assoc();
      if ($_SERVER['unix'] <= strtotime('+15 minutes', $wrongAttempts['datum_foutepoging']))
      {
        $searchForLoginLog = $link->query("SELECT * FROM `paneel_logs` WHERE `ip`='$ip' AND `bericht`='Te veel foute pogingen achter elkaar om in te loggen/registreren/wachtwoord veranderen' ORDER BY `datum_log` DESC");
        if (!$searchForLoginLog)
        {
          echo '0+Er is een fout opgetreden, kom later terug';
          exit();
        }
        else
        {
          $loginLog = $searchForLoginLog->fetch_assoc();

          if ($_SERVER['unix'] >= strtotime('+15 minutes', $loginLog['datum_log']) || $searchForLoginLog->num_rows === 0)
          {
            $insertLog = $link->query("INSERT INTO `paneel_logs` (`ip`, `bericht`, `plaats`, `datum_log`) VALUES ('$ip', 'Te veel foute pogingen achter elkaar om in te loggen/registreren/wachtwoord veranderen', 'index', '$datum')");
            if (!$insertLog)
            {
              echo '0+Er is een fout opgetreden, kom later terug';
              exit();
            }
            else
            {
              echo '0+Er is een fout opgetreden, kom later terug';
              exit();
            }
          }
          else
          {
            echo '0+Er is een fout opgetreden, kom later terug';
            exit();
          }
        }
      }
      else
      {
        $deleteWrongAttempts = $link->query("DELETE FROM `paneel_foutepogingen` WHERE `ip`='$ip'");
        if (!$deleteWrongAttempts)
        {
          echo '0+Er is een fout opgetreden, kom later terug';
          exit();
        }
      }
    }
  }
}

function login($gebruikersnaam, $wachtwoord)
{
  include($_SERVER['DOCUMENT_ROOT'].'/paneel/includes/init.php');

  checkWrongAttempts();
  getIpadress();
  $ip = sha1($_SERVER['ip']);
  $datum = $_SERVER['unix'];
  $gebruikersnaam = $link->real_escape_string($gebruikersnaam);

  $searchForUser = $link->query("SELECT * FROM `leden` WHERE `gebruikersnaam`='$gebruikersnaam'");
  if (!$searchForUser)
  {
    $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
    if (!$insertWrongLogin)
    {
      echo '0+Probeer het later opnieuw om in te loggen';
    }
    else
    {
      echo '0+Probeer het later opnieuw om in te loggen';
    }
  }
  else
  {
    if ($searchForUser->num_rows === 1)
    {
      $userDetails = $searchForUser->fetch_assoc();

      if (password_verify($wachtwoord, $userDetails['wachtwoord']))
      {
        $updateUserAccount = $link->query("UPDATE `leden` SET `laatst_online`='$datum' WHERE `gebruikersnaam`='$gebruikersnaam'");
        if (!$updateUserAccount)
        {
          echo '0+Probeer het later opnieuw om in te loggen';
        }
        else
        {
          $_SESSION['id'] = $userDetails['id'];
          echo '1+Je bent succesvol ingelogd';
        }
      }
      else
      {
        $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
        if (!$insertWrongLogin)
        {
          echo '0+Probeer het later opnieuw om in te loggen';
        }
        else
        {
          echo '0+Er zijn foute log in gegevens ingevuld';
        }
      }
    }
    else
    {
      $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
      if (!$insertWrongLogin)
      {
        echo '0+Probeer het later opnieuw om in te loggen';
      }
      else
      {
        echo '0+Deze gebruiker kan niet worden gevonden';
      }
    }
  }
}

function register($gebruikersnaam, $wachtwoord, $wachtwoordher, $emailadres)
{
  include($_SERVER['DOCUMENT_ROOT'].'/paneel/includes/init.php');

  checkWrongAttempts();
  getIpadress();
  $ip = sha1($_SERVER['ip']);
  $datum = $_SERVER['unix'];
  $gebruikersnaam = $link->real_escape_string($gebruikersnaam);
  $wachtwoord = $link->real_escape_string($wachtwoord);
  $emailadres = $link->real_escape_string($emailadres);

  $searchForUser = $link->query("SELECT * FROM `leden` WHERE `gebruikersnaam`='$gebruikersnaam'");
  if (!$searchForUser)
  {
    $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
    if (!$insertWrongLogin)
    {
      echo '0+Probeer het later opnieuw om te registreren';
    }
    else
    {
      echo '0+Probeer het later opnieuw om te registreren';
    }
  }
  else
  {
    $userDetails = $searchForUser->fetch_assoc();

    if (strlen($gebruikersnaam) >= 2 && strlen($gebruikersnaam) <= 15)
    {
      if ($searchForUser->num_rows === 0)
      {
        if ($wachtwoord === $wachtwoordher)
        {
          if (preg_match('/\A(?=[\x20-\x7E]*?[A-Z])(?=[\x20-\x7E]*?[a-z])(?=[\x20-\x7E]*?[0-9])[\x20-\x7E]{6,}\z/', $wachtwoord) && strlen($wachtwoord) > 8)
          {
            if (filter_var($emailadres, FILTER_VALIDATE_EMAIL))
            {
              $checkPostedEmail = $link->query("SELECT * FROM `leden` WHERE `emailadres`='$emailadres'");
              if (!$checkPostedEmail)
              {
                echo '0+Probeer het later opnieuw om te registreren';
              }
              else
              {
                $wachtwoord = password_hash($wachtwoord, PASSWORD_DEFAULT);

                $createUserAccount = $link->query("INSERT INTO `leden` (`gebruikersnaam`, `wachtwoord`, `emailadres`, `ip`, `registratie_datum`) VALUES ('$gebruikersnaam', '$wachtwoord', '$emailadres', '$ip', '$datum')");
                if (!$createUserAccount)
                {
                  echo '0+Probeer het later opnieuw om te registreren';
                }
                else
                {
                  echo '1+Je account is aangemaakt, je kan nu inloggen';
                }
              }
            }
            else
            {
              $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
              if (!$insertWrongLogin)
              {
                echo '0+Probeer het later om te registreren';
              }
              else
              {
                echo '0+Dit emailadres is niet geldig';
              }
            }
          }
          else
          {
            $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
            if (!$insertWrongLogin)
            {
              echo '0+Probeer het later opnieuw om te registreren';
            }
            else
            {
              echo '0+Je wachtwoord moet bestaan uit kleine en grote letters, nummers, symbolen en grote zijn dan 8 tekens';
            }
          }
        }
        else
        {
          $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
          if (!$insertWrongLogin)
          {
            echo '0+Probeer het later opnieuw om te registreren';
          }
          else
          {
            echo '0+De wachtwoorden komen niet overeen';
          }
        }
      }
      else
      {
        $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
        if (!$insertWrongLogin)
        {
          echo '0+Probeer het later opnieuw om te registreren';
        }
        else
        {
          echo '0+Deze gebruiker helaas bestaat al';
        }
      }
    }
    else
    {
      $insertWrongLogin = $link->query("INSERT INTO `paneel_foutepogingen` (`ip`, `datum_foutepoging`) VALUES ('$ip', '$datum')");
      if (!$insertWrongLogin)
      {
        echo '0+Probeer het later opnieuw om te registreren';
      }
      else
      {
        echo '0+De gebruikersnaam moet tussen de 2 en 15 tekens bestaan';
      }
    }
  }
}
?>

Ik hoor graag verbeter punten! :D

[size=xsmall]Toevoeging op 12/03/2017 18:41:16:[/size]

TOEVOEGING:
IP adres wordt gehasht met SHA1 om de volgende reden: Voor het spel waar ik dit voor maak, controleren ze op of ik ips, keyloggers etc. achterhaal. Dus van hun moest ik IP doen met SHA1 of een andere hash.




>> Maar misschien ben ik nu een beetje dom, wat is er nu dan mis met mijn script?

Zoals ik al aangaf:

Je bent nog maar pas bezig met programmeren, en dat is ook te merken. Zoals hier boven al is gezegd ... al die if's en else's achter elkaar (geneste constructies) is eerlijk gezegd nogal een puinhoop. Totaal onoverzichtelijk en totaal niet beheersbaar als je iets moet wijzigen of aanvullen.
Als validatie kan je rij met if'jes maken waarin je alle invoer controleert, en op het moment dat er iets niet okee is, dat je die foutmelding in een array plaatst. Uiteindelijk bij het versturen van de de waarin in het formulier kijk je met [php]count[/php] of er waardes zijn in je array, en zo ja, doorloop die foutmeldingen dan met foreach. Als er geen waardes in de array zijn, dan verwerk je de gegevens in de database.

Hallo!

Ik was even bezig en heb er dit van gemaakt:


<?php
function createLog($bericht, $plaats)
{
  global $_CONFIG;
  global $link;
  global $ip;
  global $datum;

  $ip = sha1($ip);

  $createLog = $link->query("INSERT INTO `".$_CONFIG['prefix']."logs` (`bericht`, `plaats`, `ip`, `datum`) VALUES ('".$link->real_escape_string($bericht)."', '".$link->real_escape_string($plaats)."', '".$link->real_escape_string($ip)."', '".$link->real_escape_string($datum)."')");
  if ($createLog)
  {
    return true;
  }
}


function login($gebruikersnaam, $wachtwoord)
{
  global $_CONFIG;
  global $link;

  $searchForUser = $link->query("SELECT `id`, `gebruikersnaam`, `wachtwoord` FROM `leden` WHERE `gebruikersnaam`='$gebruikersnaam'");
  if($searchForUser)
  {
    if ($searchForUser->num_rows === 1 && password_verify($wachtwoord, $searchForUser['wachtwoord']))
    {
      if (createLog($gebruikersnaam.' is met succes ingelogd', 'index') === true)
      {
        $_SESSION['id'] === $searchForUser['id'];
        return true;
      }
    }
  }
}
?>


kijken of niks empty is etc. gebeurt bij de login.php (Ik werk met ajax).
Is dit dan al beter? Als hij true aangeeft wordt er bij login.php reload gedaan van pagina en bij false error: Foutieve login gegevens

Checklist. ;-)

[ ] Je gebruikt te vaak global. Je bouwt daarmee via de achterdeur afhankelijkheden in. Maak die afhankelijkheden zichtbaar door variabelen op te nemen als parameters in een functie. Dit geldt bijvoorbeeld voor global $ip in createLog($bericht, $plaats): als je er nou createLog($bericht, $plaats, $ip) van maakt, maak je zichtbaar dat je het IP-adres logt.

[ ] Als je een functie bij succes verlaat met return true, is het wel zo handig (en gebruikelijk) om voor alle overige gevallen return false te gebruiken.

[ ] Je gebruikt real_escape_string() waar dat helemaal niet nodig is. Bijvoorbeeld sha1($ip) geeft je een hexadecimaal getal: dat hoef je niet te escapen.

[ ] Deze query is extreem gevoelig voor SQL-injectie; je kunt hier beter een prepared statement gebruiken:

<?php
$link->query("SELECT `id`, `gebruikersnaam`, `wachtwoord` FROM `leden` WHERE `gebruikersnaam`='$gebruikersnaam'");
?>


[ ] Waarom doe je SELECT gebruikersnaam in combinatie met WHERE gebruikersnaam='$gebruikersnaam'? Als je de gebruikersnaam al weet, hoef je die niet te selecteren.

[ ] In de functie login() mis je na $link->query(...) een fetch: je haalt het resultaat nu niet op.

[ ] Probeer if-constructies wat minder vaak te nesten:


<?php
// Niet zus:
if ($foo == true) {
    if ($bar == 'baz') {
        if ($qux == 1) {
            // ···
        }
    }
}

// Maar zo:
if (
    $foo == true
    && $bar == 'baz'
    && $qux == 1
) {
    // ···
}
?>
Hallo! Bedankt voor de reactie :D IK zal het meteen aanpassen!



[size=xsmall]Toevoeging op 16/03/2017 15:53:30:[/size]

Maar zou dit ook kunnen:


function loggedIn()
{
  if (
      !empty($_SESSION['id'])
     )
  {
    return true;
  }
  else
  {
    return false;
  }
}

Ik ben namelijk iemand die graag overal het zelfde doet :D
ikzelf zou met isset() controleren.
+ nog een vraag.
Kan ik ook niet via de functie $_CONFIG en $link meegeven? Dan heb ik ook minder globals, right?

[size=xsmall]Toevoeging op 16/03/2017 16:11:18:[/size]

+ nog een vraag.
Kan ik ook niet via de functie $_CONFIG en $link meegeven? Dan heb ik ook minder globals, right?
Waarom zou je een eigen global-variabele aanmaken? Ikzelf sla mijn instellingen in een $settings-array op.
- Ariën - op 16/03/2017 15:56:24

ikzelf zou met isset() controleren.


Zou u misschien even een voorbeeldje willen sturen? Ik snap het niet helemaal wat u bedoeld

Ik snap het al^^

[size=xsmall]Toevoeging op 16/03/2017 16:18:18:[/size]

- Ariën - op 16/03/2017 16:11:52

Waarom zou je een eigen global-variabele aanmaken? Ikzelf sla mijn instellingen in een $settings-array op.

Pas ik gelijk even aan! Was nog van vorig script
Geen empty gebruiken, maar isset!
Een voorbeeld is niet zo heel nodig, omdat je de werking ook op php.net/isset kan vinden.

Reageren