Hilfe bei beheben von SQL Injection

  • Jo.

    Man hat
    SELECT * FROM überall in vielen Tutorials gesehen.
    Deswegen haben das alle so übernommen (auch ich mal).
    Mittlerweile habe auch ich mitbekommen (aus den PHP Forum) das man trotzdem alle Spaltennamen da auflisten soll und das (*) weglassen soll.

    Ok. Aber gehört das auch zu den Fehlern die eine SQL Injection auslösen

  • Nein. ist es nicht. Warum sollten wir uns mit so einem Durcheinander auseinander setzen? Benutze konsequent PDO oder die mysqli*-Funktionen, aber nicht beides.


    Nein

    Ok Danke für deine Rückmeldung jeder sieht das ein bisschen anderst damit komme ich aber trotzdem klar. Nur müsstest du mir dabei ein bisschen helfen, da ich nicht ganz genau weis was du damit meinst und um das Thema nicht in die Länge zuziehen

  • Ich lese nochmal beides durch:thumbup:

  • Wir können ja mal oben anfangen.

    PHP
    1. <?php
    2. require('connection/db1.php');
    3. // Teilnehmerliste
    4. $s2 = $bdd->query("SELECT * FROM convoy_part WHERE user_convoy=".$_GET['id']);
    5. // Zählung der Datensätze
    6. include 'connection/sql1.php';
    7. $users_r = mysqli_query($conn, "SELECT * FROM `convoy_part` WHERE user_convoy=".$_GET['id']);
    8. $users = mysqli_num_rows($users_r);
    9. ?>

    2 mal connection include ?

    Warum?

    Wie sieht der Inhalt der beiden Datein aus ?

    Warum 2 mal die gleiche Abfrage ?

    Code
    1. $s2 = $bdd->query("SELECT * FROM convoy_part WHERE user_convoy=".$_GET['id']);
    2. $users_r = mysqli_query($conn, "SELECT * FROM `convoy_part` WHERE user_convoy=".$_GET['id']);

    Das bitte ändern.

    Hier hast du Sql injection.

    Nutze hier mysqli_real_escape_string für dein $_GET['id']

  • Also zwei Abfragen bei der einen werden die Datensätze mit der ID gezählt und die andere ruft diese ab. -> Danke für den Hinweis diese könnte ich eigentlich zusammenführen


    Nutze hier mysqli_real_escape_string für dein $_GET['id'] -> Wie genau soll ich das einsetzen?



    *Zu den 2 mal include




    db1.php

    <?php

    $bdd = new PDO('mysql:host=localhost;dbname=;charset=utf8', '', '');

    ?>


    sql1.php


    <?php

    $servername = "localhost";

    $username = "";

    $password = "";

    $database = "";

    // Create connection

    $conn = new mysqli($servername, $username, $password, $database);

    // Check connection

    if ($conn->connect_error) {

    // die("Connection failed: " . $conn->connect_error);

    }

    ?>

  • Zu den 2 mal Include.
    Wenn du PDO benutzen willst dann nutze das erste.
    Bei mysqli das zweite.
    Aber entscheide dich für eins.


    Ich weis nicht ob das so ganz stimmt

    Mit den mysli_real_escape_string meinte ich das so, das stimmt.
    Ob es aber Sicher ist, sage ich nix mehr zu.
    Dein Code

    Code
    1. $id = mysqli_real_escape_string($bdd, $_GET['id']);
    2. $s2 = $bdd->query("SELECT * FROM convoy_part WHERE user_convoy= .$id");

    Wenn ich das richtig sehe benutzt der Code die DB-Connection von der PDO include Datei.
    Ich glaube nicht das es geht, weil du mysqli nutzt (Bin mir da nicht sicher, weil habe das noch nie getestet).

    Dein durcheinander mit PDO und MYSQLI bringt nur Stress.
    LÖSCHE da jetzt eins weg und bleibe bei eins.

  • Ich weis nicht ob das so ganz stimmt

    $id = mysqli_real_escape_string($bdd, $_GET['id']);

    $s2 = $bdd->query("SELECT * FROM convoy_part WHERE user_convoy= .$id");

    Nein! Das ist gefährlich! mysqli_real_escape_string() bringt überhaupt nichts wenn du den damit behandelten Wert nicht in Anführungszeichen setzt: im Query müssen um $id dann Anführungszeichen stehen - das wurde dir aber auch schon in #3 geschrieben! Beschäftige dich erstmal mit Kontextwechseln damit du das eigentliche Problem überhaupt verstehst. Ich würde dir auch eher zu prepared statements raten und nicht die Querys von Hand zusammen bauen.


    Die restlichen Fehler in den zwei Zeilen wurden ja schon teilweise angesprochen:

    • das ».$id« gehört hinter die Klammer, so gibt das einen Query der nicht das macht was du erwartest (vom Syntax her zwar richtig, die Logik ist aber falsch)
    • in $bdd steht eine PDO-Verbindung, damit kann eine mysqli-Funktion natürlich nichts anfangen - entscheide dich endlich für eine Variante und räum den Code auf (ich würde PDO nehmen)
    • Vergiss »SELECT *« und schreibe *immer* alle Spalten hin die du brauchst
  • Nein! Das ist gefährlich! mysqli_real_escape_string() bringt überhaupt nichts wenn du den damit behandelten Wert nicht in Anführungszeichen setzt: im Query müssen um $id dann Anführungszeichen stehen - das wurde dir aber auch schon in #3 geschrieben! Beschäftige dich erstmal mit Kontextwechseln damit du das eigentliche Problem überhaupt verstehst. Ich würde dir auch eher zu prepared statements raten und nicht die Querys von Hand zusammen bauen.


    Die restlichen Fehler in den zwei Zeilen wurden ja schon teilweise angesprochen:

    • das ».$id« gehört hinter die Klammer, so gibt das einen Query der nicht das macht was du erwartest (vom Syntax her zwar richtig, die Logik ist aber falsch)
    • in $bdd steht eine PDO-Verbindung, damit kann eine mysqli-Funktion natürlich nichts anfangen - entscheide dich endlich für eine Variante und räum den Code auf (ich würde PDO nehmen)
    • Vergiss »SELECT *« und schreibe *immer* alle Spalten hin die du brauchst

    Erstmal Danke für deinen Beitrag. Ich werde nun versuchen alles in PDO umzuwandeln.

  • 2. warum bist du jetzt raus. Was habe ich dir getan?

    Das kann ich dir sagen.
    Seid Mittwoch haben dir 3 Leute (jetzt schon 4) gesagt was du änderst musste.
    Du machst da ja ständig was aber immer nur das, was noch sinnlos ist.
    Deswegen arbeite jetzt alle Punkte von #1 bis 37# ab und zeige uns dann deinen neuen Code.
    NUR PDO ,

    oder NUR mysqli
    Ohne leere Abfragen, die kann Sinn haben.
    Mehrfaches include von Db-Connection vermeiden
    usw…
    Erst dann wieder melden, mit neuem Code.