Sicherheit Myqli und Formulare


Lukas03

Mitglied
Hallo,

ich mache mir Gedanken über die Sicherheit von Formularen und eine mögliche SQL Injection.

Ich arbeite mit der Mysqli Extension (Für mich einfacher) möchte allerdings dabei auch bleiben, und erstmal keine Prepared-Statements nutzen.

Sollte ich etwas ändern oder so belassen?
Achso das Formular funktioniert wunderbar auf localhost mit xampp.


Hier mal mein Code:

PHP:
<form action="form.php" method="post">
  <label for="name"></label>
  <input type="text" name="name" id="name">
  <input type="submit" name="senden" id="senden" value="Senden">
</form>


<?php
include ('db.php');
$name = stripslashes ($_POST['name']);
$name = mysqli_real_escape_string($db,$name);

if (isset($_POST['senden'])){
         $insertdb = mysqli_query($db,"INSERT INTO user (name) VALUES ('$name')");  }
      

 ?>
 

ComFreek

Mod | @comfreek
Moderator
  • Warum benutzt du stripslashes?
  • Du prüfst nicht, ob $_POST['name'] definiert ist.
  • Dein Formular ist nicht CSRF-sicher.
Allerdings bezüglich SQL Injections sieht es in Ordnung aus.

Ich arbeite mit der Mysqli Extension (Für mich einfacher) möchte allerdings dabei auch bleiben, und erstmal keine Prepared-Statements nutzen.
Grundsätzlich lebst du so ein für SQL Injection anfälligeres Leben :)
 

Lukas03

Mitglied
  • Warum benutzt du stripslashes?
  • Du prüfst nicht, ob $_POST['name'] definiert ist.
  • Dein Formular ist nicht CSRF-sicher.

  • Warum benutzt du stripslashes?
  • Du prüfst nicht, ob $_POST['name'] definiert ist.
  • Dein Formular ist nicht CSRF-sicher.
Allerdings bezüglich SQL Injections sieht es in Ordnung aus.


Grundsätzlich lebst du so ein für SQL Injection anfälligeres Leben :)
Hallo @ComFreek,

ersteinmal besten Dank für die Hilfe, hab versucht die Punkte abzuarbeiten, die du mir empfohlen hast.
Die stripslashes habe ich auf einem tutorial gehabt, jetzt wurden sie entfernt..
Ich hoffe ich habe es richtig verstanden, den Code habe ich überarbeitet hier meine Lösung :

PHP:
<?php
session_start();
include ('db.php');


if (empty($_SESSION['key']))
    $_SESSION['key'] = bin2hex(random_bytes(32));
    $csrf = hash_hmac('sha256', 'string: index.php', $_SESSION['key']);
   
    if (isset($_POST['submit'])){
        if (hash_equals($csrf, $_POST['csrf'])) {
               
         }
         else
        echo 'CSRF Token Fehler';
                }

$username = $db -> real_escape_string($_POST['username']);




if (isset($_POST['submit'])){
       
    if (empty($username)){

        echo "Bitte einen Usernamen angeben!";
       
        }
       
        else{
           
            $insertdb = mysqli_query($db,"INSERT INTO user (username) VALUES ('$username')");
            }
                }

?>


<form method="post" action="token.php">
<input type="text" name="username" placeholder="Wie ist dein Username?">
<input type="hidden" name="csrf" value="<?php echo $csrf ?>">
<input type="submit" name="submit" value="SUBMIT">
</form>
 

m.scatello

Erfahrenes Mitglied
Du solltest deinen Quellcode ordentlich einrücken, damit er besser lesbar ist.

Zu deinem QUellcode siehe Kommentare:
PHP:
<?php
  session_start();
  include ('db.php');


  if (empty($_SESSION['key']))
     $_SESSION['key'] = bin2hex(random_bytes(32));
     
  $csrf = hash_hmac('sha256', 'string: index.php', $_SESSION['key']);
   
  if (isset($_POST['submit'])) 
  {
     if (hash_equals($csrf, $_POST['csrf'])) 
     {
         // Was soll das hier???
     }
     else
        echo 'CSRF Token Fehler';
  }

  // Hier greifst du immer noch auf $_POST['username'] zu, 
  // ohne zu wissen, ob die Variable existiert
  
  $username = $db -> real_escape_string($_POST['username']);

  if (isset($_POST['submit']))
  {    
     if (empty($username))
     {
        echo "Bitte einen Usernamen angeben!";
     }
     else
     {
        // Hier benutzt du den prozeduralen Stil. aber bei real_escape_string
        // den objektorientierten Stil, das kann nicht funktionieren
        $insertdb = mysqli_query($db,"INSERT INTO user (username) VALUES ('$username')");
        
        // Hier fehlt eine Abfrage, ob das Insert auch wirklich funktioniert hat
        // Siehe mysqli_errno / mysqli_error
     }
  }

?>


<form method="post" action="token.php">
<input type="text" name="username" placeholder="Wie ist dein Username?">
<input type="hidden" name="csrf" value="<?php echo $csrf ?>">
<input type="submit" name="submit" value="SUBMIT">
</form>
 

Lukas03

Mitglied
Danke @m.scatello für die Hilfestellung :)

// Was soll das hier???
Ist nur eine kontrolle für die sessionübergabe "echo $_SESSION['username'];"


// Hier benutzt du den prozeduralen Stil. aber bei real_escape_string // den objektorientierten Stil, das kann nicht funktionieren
Funktioniert aber trotzdem, erhalte einen neuen Beitrag in der DB
Hab den Code wieder überarbeitet, bin weiterhin Dankbar für jeden Tipp :)


PHP:
<?php
session_start();
include ('db.php');

if (empty($_SESSION['key']))
    $_SESSION['key'] = bin2hex(random_bytes(32));
    $csrf = hash_hmac('sha256', 'string: index.php', $_SESSION['key']);
    
if (isset($_POST['submit'])){
if (hash_equals($csrf, $_POST['csrf'])) {
// Prüfe user echo $_SESSION['username'];
}
    else
        echo 'CSRF Token Fehler';
                }


if (isset($_POST['submit'])){

$username = $db -> real_escape_string($_POST['username']);
if(!empty($username)){
      echo "$username wurde engegeben";
      $insertdb = mysqli_query($db,"INSERT INTO user (username) VALUES ('$username')");
      echo $db->errno.'-'.$db->error;}
else{
      echo "Bitte einen Usernamen angeben!";
}}

?>
 

Lukas03

Mitglied
So? Oder reden wir aneinander vorbei?

Code:
$username = mysqli_real_escape_string($db, $_POST['username']);

Prüfung auf Fehler
Code:
printf("Errorcode: %d\n", mysqli_errno($db));
 

ComFreek

Mod | @comfreek
Moderator
Und wieder ist die Formatierung durch den Wind. :(
Ich weiß gar nicht, wie man selbst damit arbeiten kann.

echo "$username wurde engegeben";
Erstens ist $username überschrieben worden mit der MySQLi-escapedten Version von $username und daher die Nachricht semantisch falsch. Und zweitens gibt man niemals Variablen ohne HTML-Escaping in einem HTML-Kontext aus. Sonst hast du wie hier eine HTML Injection (aka XSS) Sicherheitslücke.
Ich lass dich mal daran arbeiten, wie du beide Probleme beseitigen kannst :)

Zu deinem CSRF-Code kann ich nicht viel sagen: ich weiß nicht, ob die Generierung des Tokens passt. Was ich jedoch sagen kann, ist, dass dein Token zu langlebig ist. Es gibt keine Zeitbeschränkung, ab wann es abläuft. Das ist alles nicht trivial. Wenn du ein echtes System entwickelst, nutze eine Library dafür.
 

Lukas03

Mitglied
Erstens ist $username überschrieben worden mit der MySQLi-escapedten Version von $username und daher die Nachricht semantisch falsch. Und zweitens gibt man niemals Variablen ohne HTML-Escaping in einem HTML-Kontext aus. Sonst hast du wie hier eine HTML Injection (aka XSS) Sicherheitslücke.
Ich lass dich mal daran arbeiten, wie du beide Probleme beseitigen kannst
Die Session Username kommt aus dem Login diese wird nicht überschieben, das mit dem HTML -Escaping werde ich genauer unter die Lupe nehmen.

Zu deinem CSRF-Code kann ich nicht viel sagen: ich weiß nicht, ob die Generierung des Tokens passt. Was ich jedoch sagen kann, ist, dass dein Token zu langlebig ist. Es gibt keine Zeitbeschränkung, ab wann es abläuft. Das ist alles nicht trivial. Wenn du ein echtes System entwickelst, nutze eine Library dafür.
Sobald sich die PHPSESSID im Browser ändert wird mir die Fehlermeldung "CSRF Token Fehler" angezeigt


mysqli_query($db,"INSERT INTO user (username) VALUES ('$username')") or die ("MySQL-Error: " . mysqli_error($db));
Danke, hab ich nun eingetragen, sieht gleich besser aus :)
 

ComFreek

Mod | @comfreek
Moderator
Der Username kommt laut deinem Code aus $_POST und wird auch überschrieben, bevor du ihn via echo ausgibst:
PHP:
$username = $db -> real_escape_string($_POST['username']);
if(!empty($username)){
  echo "$username wurde engegeben";
Sobald sich die PHPSESSID im Browser ändert wird mir die Fehlermeldung "CSRF Token Fehler" angezeigt
Ah, da hast du recht. Stimmt, wenn du die PHPSESSID nirgends gegenüber Dritten leakst, brauchst du nicht zwingend einen Expiry Timeout. Siehe auch What's a good time period before refreshing CSRF token of the user session? und How frequent should the Token Updation in CSRF security be?.

mysqli_query($db,"INSERT INTO user (username) VALUES ('$username')") or die ("MySQL-Error: " . mysqli_error($db));
Das Ausgeben von genauen Fehlermeldungen ist i. Allg. schlechte Praxis. Im besten Fall verwirrst du normale Nutzer. Im schlimmsten Fall leakst du sensible Daten, die es einem Angreifer noch einfacher zu machen, für deine bestimmte Serverkonfiguration oder SQL-Queries Lücken zu finden.
 

Neue Beiträge

Forum-Statistiken

Themen
272.361
Beiträge
1.558.639
Mitglieder
187.834
Neuestes Mitglied
jordanx0206