db_query funktion richtig nutzen

[Aradiv]

Das VMS hat eine schöne Queryfunktion die wenn man sie richtig anwendet alle Variablen mit mysql_real_escape_string für SQL Injections unbrauchbar macht. Leider wird diese Funktion NIRGENDS im VMS wirklich verwendet.

Nehmen wir erstmal die db_query funktion auseinander
dies ist die gesamte funktion

function db_query($sql_tag){ 
global $count_query; 
    $count_query    ++; 
    $fargs = func_get_args(); 
 
  if (!empty($fargs)){ 
  $vargs = array(); 
    foreach($fargs as $key => $arg){ 
      $vargs[$key] = mysql_real_escape_string($arg); 
    } 
    array_shift($vargs); 
    if(!empty($vargs))$sql_tag = vsprintf($sql_tag,$vargs); 
  } 
    if($ret = mysql_query($sql_tag)){ 
      return $ret; 
    }else{ 
      return 0; 
    } 
} 

Zu erst betrachten wir mal die Variablen die zur verfügung stehen oder erzeugt werden.

function db_query($sql_tag){ 
global $count_query; 
    $count_query    ++; 
    $fargs = func_get_args(); 

$sql_tag ist unser Query String die vorerst einzige Variable die an die Funktion db_query übergeben werden kann.

$count_query zählt die querys bei einem Aufruf der Seite (ist für uns unintressant)

$fargs ist ein array mit allen Variablen die an die Funktion db_query übergeben werden.

also dann weiter mit der Logik

Moment "alle Variablen"?
Ja richtig alle Variablen mit func_get_args wurde aus der funktion db_query mit 1 Variablen eine funktion mit beliebig vielen Variablen also können wir db_query auch so aufrufen

db_query($sql_tag, $var1, $var2, $var3, $var4, $var5, $var6, $var7 ...) 

Ok jetzt aber wirklich zur Logik

 if (!empty($fargs)){ 
  $vargs = array(); 
    foreach($fargs as $key => $arg){ 
      $vargs[$key] = mysql_real_escape_string($arg); 
    } 
    array_shift($vargs); 
    if(!empty($vargs))$sql_tag = vsprintf($sql_tag,$vargs); 
  } 

Zu erst wird überprüft ob $fargs leer ist. (kann eigentlich nicht sein den db_query leer aufzurufen also nur db_query() macht wenig Sinn

ok danach erzeugen wir ein leeres Array $vargs;
jetzt wird jede Variable aus $fargs einzeln genommen mit mysql_real_escape string behandelt und unter dem selben key (nummer von 1 bis x) in $vargs abgelegt.

array_shift nimmt das erste Element von $vargs gibt es zurück und löscht es aus dem Array $vargs
jetzt wird noch überprüft ob $vargs nach dem löschen des einen Elemnts leer ist und danach wird $sql_tag mit vsprintf erzeugt.

Was passiert bei vsprintf?
vsprintf gibt einen Formatierten strung zurück und nimmt als 2. Parameter ein Array mit den ersetzungen entgegen.

heißt aus
vsprintf("%d-%d", array(7, 3)) wird eine 7-3
aus vsprinf("Hallo %s", User) wird Hallo User
usw

Danach wird nurnoch das Query ausgeführt mit mysql_query.

So was bringt uns unser wissen jetzt?

Nehmen wir mal an wir haben folgende abfrage

db_query("SELECT * FROM vms_kontodaten WHERE uid=$uid"); 

1. wenn man $uid maniplieren kann dann wären sql injections tür und tor geöffnet
2. was steht den in $uid wahrscheinlich eine Zahl aber sicher?
um das abzusichern könnte man jetzt $uid mit intval($uid) behandeln oder bei der zuweißung ein int casten aber ist das beim einsetzen in die db_query funktion immernoch eine Zahl oder hat sich das inzwichen verändert? Sicher weiß man das so nicht.
Also was tuen?
Nochmal mit intval, eregi und anderen prüfen?
Nein warum denn wir haben doch unsere db_query methode

Dort machen wir aus

db_query("SELECT * FROM vms_kontodaten WHERE uid=$uid"); 

einfach

db_query("SELECT * FROM vms_kontodaten WHERE uid=%01d", $uid); 

So was passiert jetzt?
der platzhalter %01d wird mit der ZAHL %uid ersetzt und wenn $uid keine Zahl ist wird eine 0 eingesetzt und vorher mit mysql_real_escape_string behandelt

heißt ausdrücke wie "123456' OR 1=1" werden unbrauchbar gemacht.

So jetzt das erschreckende bzw Fragwürdige.
Warum gibt es diese Funktion im VMS wenn sie nie genutzt wird?

mfg
Aradiv

[jpwfour]

...
Warum gibt es diese Funktion im VMS wenn sie nie genutzt wird?
...

Wildes Copy&Paste.

Die Funktion hat ja auch schon eine "Geschichte" und war afaik in 3 Versionen drin, da sie Anfangs ja eher mehr zu Fehlern führte (%-Zeichen), als dass sie sinnvoll war. Jetzt ist sie drin und wird halt nirgends genutzt, da würde auch eine simplere Funktion reichen, die nur mit 1 Argument arbeitet.

Alles jetzt dafür umzuschreiben wär eingies an Arbeit, und bei sovielen Änderungen schlecihen sich gern Fehler ein, d.h. alles müsste wieder getestet werden usw.

Aber kannst gerne machen

[tweetymr]

Sry wenn ich den Thread nochmal ausgrabe aber ich finde, dass passt zu meiner Frage

Und zwar bin ich grade dabei die Performance meiner Seite ein bisschen zu verbessern.
Jetzt stellt sich mir die Frage: Wann MUSS/SOLLTE ich db_query einsetzen um gegen Injections geschützt zu sein und wann reicht ein normaler mysql_query, da dieser schneller ausgeführt werden kann?


// Offtopic
Und gibt es irgendwelche Ideen die Performance und die Sicherheit noch zu erhöhen?
Habe hier schon im Forum gesucht aber soweit eigentlich nix brauchbares gefunden.
Habe das VMS1.2.4 falls das eine Rolle spielen sollte
// Offtopic

[Lokutos]

grundsätzlich könnte man die buchungsliste anpassen das nicht jedesmal n code generiert werden muss da würde n auto-increment reichen.

dan kann man ja langsam die dateien durchgehen und das create_code() entfernen.


alles andere ist halt wenn 1 query geht aus mehreren zusammenfassen is meist besser.

und halt auf unnötige abfragen verzichtn (wenn ich den menüs schon der kontostand ausgelesen wird muss man ihn nicht noch in der kontoübersicht anfragen und in der auszahlen.php und all so zeugs wenn was vorhanden ist kann mans ja nutzen.

Natürlich ist das eher negative wenn du ständig irgendwas aus und einbust.

mfg Lokutos

[jpwfour]

...db_query einsetzen um gegen Injections geschützt zu sein und wann reicht ein normaler mysql_query, da dieser schneller ausgeführt werden kann?
...

Das stimmt zwar in etwa, dass db_query langsamer ist als mysql_query, aber nur in einem so verschwindend geringen Bereich, da kann man allein durch weglassen eines einzigen Querys im Script ca. 1 Mio fach mehr Rechenzeit einsparen

Also du kannst an jeder Stelle db_query() benutzen.


Performance kann auch durch verkleinern von Grafiken verbessert werden, was zusätzlich Traffic spart.

Hast du die möglichkeit, dir ein Log vom MySQL Server generieren zu lassen?

Sonst ist Optimierung immer viel Eigenarbeit :-(

[tweetymr]

Ok, also kann ich überall db_query einsetzen. Find ich schonmal ganz nett^^ Schon wieder was zu tun xD

Das mit dem create_code hätte mir auch kommen können, danke für den Tipp

jpwfour: Wenn du mir sagst wie ich so einen Log ausspucken lassen kann werde ich das mal mache Habe Root, also dürfte kein Problem sein =)

Ich weiß, dass es sehr viel Eigenarbeit ist aber davor scheut man sich als guter Webbi ja bekanntlich nicht ;D
Gibt es noch ein paar Anlaufstellen wo man über PHP oder MySQL Optimierung was lasen kann?
Die Benchmarks habe ich mir schon angeguckt, haben auch schon ein bisschen was gebracht

Danke für eure Beiträge bisher (Sollte man den Thread vllt mal splitten? Hat ja nix mehr wirklich mit db_query direkt zu tun :x)

LG
TweetyMR

[marcaust]

Dort machen wir aus

db_query("SELECT * FROM vms_kontodaten WHERE uid=$uid"); 

einfach

db_query("SELECT * FROM vms_kontodaten WHERE uid=%01d", $uid); 

So was passiert jetzt?
der platzhalter %01d wird mit der ZAHL %uid ersetzt und wenn $uid keine Zahl ist wird eine 0 eingesetzt und vorher mit mysql_real_escape_string behandelt

heißt ausdrücke wie "123456' OR 1=1" werden unbrauchbar gemacht.

Das ist so oder so sehr gefährlich da mysql_real_escape_string fast gar nicht greift. In dem oben aufgeführten Ausdruck befindet sich ein ' weswegen mysql_real_escape_string greift. lassen wir das ' (der Ausdruck ist ja eine Usereingabe...) einfach weg haben wir:
123456 OR 1=1
hier greift weder mysql_real_escape_string noch vsprintf. Das Ergebnis ist also:

SELECT * FROM vms_kontodaten WHERE uid=123456 OR 1=1

Der Fehler liegt in den Fehlenden ' innerhalb der Query. Machen wir aus:

db_query("SELECT * FROM vms_kontodaten WHERE uid=$uid"); 

einfach ein

db_query("SELECT * FROM vms_kontodaten WHERE uid='$uid'"); 

greift schon mal mysql_real_escape_string und macht daraus ein:
SELECT * FROM vms_kontodaten WHERE uid=\'123456 OR 1=1\' und die Funktion gibt als $sql_tag: SELECT * FROM vms_kontodaten WHERE uid='123456 OR 1=1' zurück.

Solange man:

db_query("SELECT * FROM vms_kontodaten WHERE uid=%01d", $uid); 

nur mit dem Platzhalter: %01d (für int) verwendet ist das auch sicher. Die Rückgabe hier:
SELECT * FROM vms_kontodaten WHERE uid=123456
das "or 1=1" wird einfach weg gelassen.

Gefährlich wird es wenn man einen anderen Platzhalter verwendet z.Bsp.: %01s (für Strings).

Hier wird daraus auch wieder ein:
SELECT * FROM vms_kontodaten WHERE uid=123456 OR 1=1
Hier wird wieder alles umgangen.
Besser:

db_query("SELECT * FROM vms_kontodaten WHERE uid='%01d'", $uid); 

auch hier hab ich lediglich die beiden ' eingefügt. Schon sieht das Ergebnis im $sql_tag wieder so aus:
SELECT * FROM vms_kontodaten WHERE uid='123456 OR 1=1'
also ungefährlich.

Wie man leicht sieht, ist die richtige Schreibweise der Query an sich auch von entscheidener Bedeutung. Zudem muss man davon ausgehen das mit der Funktion nicht nur Zahlen sondern auch Text abgesichert werden soll.

Wer die beiden ' um eine Variable vergisst hebelt die Sicherungen der Funktion selber aus. Ein Umstand den ich leider bei einigen Addons, etc. gesehen habe.

Aus diesem Grund sollte man Grundsätzlich immer! am Anfang eines Script jede! Usereingabe Prüfen. Das kombiniert mit der richtigen Anwendung der Funktion db_query und man ist relativ sicher.

Wer damit mal etwas testen möchte, das geht einfach mit dem Script:

<?

// Datenbankserver
$db_host        = "localhost";                             
// Mysql User
$db_user        = "xxxxx";      // Datenbank User                              
// Mysql PW
$db_pass        = "Passwort";                                    
// Datenbank
$db_base        = "Datenbank Name";                                    
// Tabellenpräfix (wichtig wenn mehrere VMS in einer DB liegen)
$db_prefix        = "vms";                                    

function db_connect() {
global $db_host,$db_user,$db_pass,$db_base,$sql_open;
    $sql_open    = mysql_connect($db_host,$db_user,$db_pass) or die('Verbindung zum Mysql Server fehlgeschlagen!');
    $sql_base    = mysql_select_db($db_base) or die("Keine oder falsche Datenbank gewählt!");
}

function db_query($sql_tag){ 
global $count_query; 
    $count_query    ++; 
    $fargs = func_get_args(); 
 
  if (!empty($fargs)){ 
  $vargs = array(); 
    foreach($fargs as $key => $arg){ 
      $vargs[$key] = mysql_real_escape_string($arg); 
          echo "<br>";
        echo "vargs[key]: ".$vargs[$key];
        echo "<br>";
    } 
    array_shift($vargs); 
    if(!empty($vargs))$sql_tag = vsprintf($sql_tag,$vargs); 
        echo "<br>";
        echo "sql_tag: ".$sql_tag;
        echo "<br>";
  } 
    if($ret = mysql_query($sql_tag)){ 
      return $ret; 
        echo "ret: ".$ret;
        echo "<br>";
    }else{ 
      return 0; 
    } 
}
$uid = "123456 OR 1=1";
//$uid = "1234";
//$uid = (int)$uid;
echo "uid: ".$uid;
echo "<br>";
db_connect();
//$sql = db_query("SELECT * FROM vms_kontodaten WHERE uid='$uid'"); 
//$sql = db_query("SELECT * FROM vms_kontodaten WHERE uid=$uid"); 
//$sql = db_query("SELECT * FROM vms_kontodaten WHERE uid=%01d", $uid);  //Zahlen
$sql = db_query("SELECT * FROM vms_kontodaten WHERE uid='%01d'", $uid);  // Zahlen
//$sql = db_query("SELECT * FROM vms_kontodaten WHERE uid=%01s", $uid);  // String
$sql = db_query("SELECT * FROM vms_kontodaten WHERE uid='%01s'", $uid);  // String
$kontoinfo = mysql_fetch_array($sql);
print_r($kontoinfo);
echo "<br>";

echo "kontoinfo".$kontoinfo['uid'];
?>

zum ändern des Aufrufs einfach die entsprechende Zeile aktivieren

[jpwfour]

...
jpwfour: Wenn du mir sagst wie ich so einen Log ausspucken lassen kann werde ich das mal mache Habe Root, also dürfte kein Problem sein =)
...

Eine kurze Anleitung dazu gibt es im Tutorial: http://www.vms-tutorial.de/wiki/Datenbank_optimieren