[ljungi]

Hi,

ich überlege mir gerade, wie ich mittels einer Methode eine SQL-Injection verhindern kann.
Also nicht eine Methode, die ich bei jedem einzutragenden Wert nutze, sondern die einen kompletten Query incl eventueller Subqueries, Joints und Unions "scannt" und eben den Standard

PHP-Code:

  private function secure($value) {

   if(get_magic_quotes_gpc())
     $value = stripslashes($value);

   if(!is_numeric($value))
     $value = "'".mysql_real_escape_string($value)."'";

    return $value;
  } 


anwendet.

Ich würde das mit pattern angehen - allerdings wollte ich erstmal eure Meinung hören, ob das von der Performance her überhaupt zu empfehlen wäre.

Ich bin unschlüssig, da ich denke, dass dann pro Query eben ne menge Arbeit auf den Interpreter zukommt.

[robo47]

wie wäre es einfach alle "fremden" werte sauber zu escapen mittels mysql_real_escape_string ?

[Basti]

Hi.

Erstmal ist deine "Standard"-Methode Käse, denn die eine Operation wird (zumindest so ähnlich) gemacht, um eingehende Werte wieder in den Zustand zu versetzen, in dem sie eigentlich erwatet würden und die zweite betrifft dann das Escaping für eine MySQL-Query. Gehört also mal garnicht zusammen.

Weiter ist das so nicht umsetzbar, wie du dir das vorstellst (wenn ich dich richtig verstehe). Das Ding ist doch, dass deine Queries eine bestimmte Struktur haben, in die Werte eingepflanzt werden. Damit die Werte die Struktur nicht ändern, verwendest du eben mysql_real_escape_string(). Wenn du nun erst die Werte einsetzt, um dann die Struktur zu analysieren, haben die Werte die Struktur womöglich bereits verändert.

Ich benutze einfach eine query-Methode in der Datenbank-Klasse, der ich a) die Query mit Platzhaltern und b) ein Array mit den Werten übergebe. Siehe auch PDO und mysqli (prepare).

Basti

[ljungi]

Danke,

werde es also wie üblich machen und jeden Wert einzeln abfragen.

@Basti,
habe mir vorhin während meines Posts was über PDO durchgelesen. Scheint sinnvoll zu sein, allerdings müsste ich dafür mein gesamtes System umkrempeln - was mir jetzt zu viel Arbeit wäre.

Für das nächste Projekt werd ich mir das mal überlegen, auch das mit dem Query mit Platzhaltern klingt gut - ist aber auch wieder ne Sache der Uumgewöhnung

[Basti]

Also ich gewöhne mich gerne daran, einen Funktionsaufruf auszulagern, den ich andernfalls Dutzende von Malen von Hand aufrufen müsste.

Basti

[Artemis]

Zitat von Basti

a) die Query mit Platzhaltern und b) ein Array mit den Werten übergebe

Wäre dann eine Funktion, die so aussieht ok/vernünftig:

PHP-Code:

<?php

function get_secure_query_string($query, $values)
{
    $secure_values = Array();
    $secure_values[] = $query;
    foreach($values as $value)
    {
        $secure_values[] = mysql_real_escape_string($value);
    }
    $secure_query_string = call_user_func_array('sprintf', $secure_values);
    return $secure_query_string;
}

?>

Die wird dann meinetwegen so aufgerufen:

PHP-Code:

<?php

$secure_query_string = get_secure_query_string("UPDATE `table` SET `name` = '%s', `password` = '%s', `alter` = %d WHERE `id` = %d", $_POST['name'], sha1($_POST['password']), intval($_POST['alter']), intval($_POST['id']));

?>

[Basti]

Diesen Extra-Schritt kannst du dir doch komplett sparen.

Bei mr sieht das verkürzt so aus:

PHP-Code:

class MysqlConnection implements IConnection
{
    public function query($sQuery, $aValues = array())
    {
        $aValues = array_map('mysql_real_escape_string', $aValues);

        $sQuery = vsprintf($sQuery, $aValues);

        $this->rResult = mysql_query($sQuery, $this->rHandle);

        // ...
    }
}


class Dao_User ...
{
    public function get($sUserName)
    {
        $sQuery = "
            SELECT
                username,
                password,
                email,
                sex,
                title,
                first_name,
                last_name,
                street_address,
                zip,
                city,
                phone,
                fax

            FROM
                users

            WHERE
                username = '%s'
            ";

        $this->Connection->query($sQuery, array($sUserName));

        return $this->Connection->fetch();
    }
} 


Basti

[Byrel]

Ehrlich gesagt wenn man jetzt nicht unbedingt die schier unendlichen mysqli_*, oci_* ... Funktionen benötigt, weil man total auf eine Datenbank optimiert so sollte auf jeden Fall PDO verwendet werden. PDO bietet für fast alle Datenbanken native Treiber an.

Bei PDO hast du auch Prepared Statements (mysqli überigens auch). Verwendest du die und verwendet deine Applikation die gleiche Zeichenkodierung wie die Datenbank so hast du alles gemacht um dich vor SQL Injections zu schützen.

Sachen mit mysqli_real_* sind IMHO nervend. Außerdem ist die Gefahr groß einmal mysql_real_* zu vergessen und schon hast du ne Schwachstelle.

MfG Byrel

[Basti]

...wenn du ein DAOs, wie das hier oben benutzt, kannst du die Connection-Klasse dahinter beliebig austauschen (mysql -> mysqli -> pdo -> ...). Wichtig ist halt, dass du TransferObjects zurück gibst, die vom eigentlich zurückgegebene Datenformat unabhängig machen und das Ergebnis hinter einer einheitlichen Schittstelle kapseln.

Basti