Aufnahme der Funktion checkquery

[k00ni]

Setzt ein Prepared-Statement ab und reinigt vorher die übergebenen Parameter mittels mysql_real_escape_string. Jede SQL-Operation von uns läuft über diese Funktion. Damit macht man es jemanden fast unmöglich, die SQL-Datenbank zu kompromitieren.
 

\';
return $S_sql_query;
}
?>

 
 
(Anmerkung: Hier gibt es noch keine Überprüfung, ob dieses magic_quotes_gpc oder so ähnlich, auf 1 oder 0 steht.)

[Powie]

Beispiel!?

[Powie]

ah habs selbst rausgefunden  [/uploads/emoticons/icon_e_surprised.gif.a005678239f11b45b64b526b2c82e9a1.gif\" alt=\":o\" />]
 
$sql = \"Insert Into bla values (\'%s\',\'%s\',%s,\'%s\') Where x = \'%u\' \";
echo check_query($sql,\"hallo1\",\"hallo2\",\"hallo3\",\"hallo4\",\"5\");

[k00ni]

Nicht häääää  /uploads/emoticons/icon_e_surprised.gif.a8707b3f35a569cb4cfe563fc72ef78d.gif\" alt=\":-o\" />/uploads/emoticons/icon_e_surprised.gif.a8707b3f35a569cb4cfe563fc72ef78d.gif\" alt=\":-o\" />
 

 Setzbar in friends/config.inc.php)
function check_query ( )
{
   global $Bo_check_chars;
// Alle Parameter, die an die Funktion übergeben wurden, zwischenspeichern.
$A_function_arguments = func_get_args( );
// Ersten Paramter als Query abspeichern und aus dem Array löschen.
$S_sql_query = array_shift( $A_function_arguments );
// Parameter werden durch die Funktion mysql_real_escape_string behandelt
// und modifiziert abgespeichert.
if (get_magic_quotes_gpc() == 0)
   {
       $A_function_arguments = array_map(\'mysql_real_escape_string\',
                                         $A_function_arguments );
}
// Ersetzt die Platzhalter im Query durch die Funktionsparameter.
$S_sql_query  = vsprintf($S_sql_query, $A_function_arguments);
   if ($Bo_check_chars === true)
   {
       $S_exemplar = \'/^[a-zA-Z09äöüÄÖÜß_]+$./*\';
       // Wenn Zeichen gefunden werden, die nicht in $S_exemplar enthalten sind
       // dann wirft er eine Exc_Wrong_Parameter-Exception. (Code 1)
       if (!preg_match( $S_exemplar, $S_sql_query))
       {
           // Code 1: Exception enthält ungültige Zeichen.
           throw new Exc_Wrong_Parameter (func_get_args(), 1);
       }
   }
   // echo $S_sql_query.\'\';
return $S_sql_query;
}
?>

 
 
Nur testweise, enthält noch Fehler bei den regulären Ausdrücken.

[k00ni]

 
Hier wird eine Variable abgefragt, nämlich Bo_check_chars, die in einer config.inc.php gesetzt wird und die sagt, ob man das SQL-Query validieren soll. Wird es validiert und werden Zeichen gefunden, die in S_exemplar nicht drin sind, wird eine Exception geworfen. Dies bewirkt ein sauberes Abbrechen des Skriptes, das SQL wird nicht ausgeführt und man kann in einem entsprechenden catch-Zweig reagieren.

[Powie]

Naja.... ich verstehe die Arbeitsweise dieser Funktion schon, aber nicht den Sinn.... wieso sollte man die brauchen. Wenn ich Daten einfliessen will die so stark eingeschränkt sein sollen, dann kann ich das auch vorher filtern.
Weshalb gibt es diese nun?

[k00ni]

Eben. Du brauchst deine Daten nicht immer filtern, sondern kannst auch mit \"unsicheren\" Quellen arbeiten. Der Sinn darin ist, dass man erstens aus Integern keine Strings mehr machen kann, also dass man eine User-ID per GET übergibt und ein Angreifer da einen String draus macht. Also die Variablen bleiben bei ihrem ursprünglichen Typ. Dies wird mit den Plathaltern erreicht.
Das Neue ist die Whitelist. Hier muss ich sagen, dass ich mich mit regulären Ausdrücken noch nicht sooooo stark beschäftigt habe. Man benötigt in den meisten Abfragen keine Sonderzeichen, wie Anführungsstriche oder sowas. Die Frage wäre jetzt, ob man da nicht das gesamte Query nimmt, sondern nur die Parameter gegen die Whitelist validiert. Weiß nicht.
Bevor ich es vergesse. In deiner SQL-Klasse die Funktion query filtert rein garnichts.
 

last_query = $query;
       $this->result = @mysql_query($query, $this->serverid);
++$this->querycount;
       if ( mysql_error($this->serverid) ) {
   echo \"PSYSDB Error in Query: $query
\";
   echo mysql_error($this->serverid);
   return false;
}
       //affected rows
if ( preg_match(\"/^\\\\s*(insert|delete|update|replace) /i\",$query) ) {
   $this->rows_affected = mysql_affected_rows($this->serverid);
   // insert_id
   if ( preg_match(\"/^\\\\s*(insert|replace) /i\",$query) ) {
      $this->insert_id = mysql_insert_id($this->serverid);
   }
   // rows affected
   $return_val = $this->rows_affected;
} else {
   $this->num_rows = mysql_num_rows($this->result);
   // Return number of rows selected
   $return_val = $this->result;
}
return $return_val;
}
?>

 
 
Wenn ich mich nicht irre, bekommst du ein Problem, wenn die diese automatische Maskierung von Sonderzeichen deaktiviert haben, was mit magic_quotes gesetzt wird, glaub ich. Es wird das übergebene Query direkt ausgeführt.
Man kann bei dieser Problematik überall ansetzen. Ob man nun in jeder Datei seine Filter auf die Parameter ansetzt oder ob man in einer Funktion alles reinbaut, sei dahin gestellt. Ich selbst favourisiere die Methode der zentralen \"Anlaufstelle\". Da man dort einmal etwas ändert und es überall gleich funktioniert.
Bei mir habe ich sogar noch eine zweite Schicht vor der SQL-Funktion. Nämlich jeweils immer eine PHP-Funktion, die das SQL-Statement vorbereitet, dann an die check_query übergibt und diese es dann absetzt.
Hier ein Beispiel aus dem aktuellen Build:
 

serverid))
   {
       // Code: 1 -> Query konnte nicht ausgeführt werden.
       throw new Exc_Query ($S_sql_query, 1);
}
return true;
}
?>

 
 
Ich prüfe da erst die Parameter auf ihren vorgesehenen Typ. Dies ginge in der check_query nur sehr umständlich liese sich meiner Meinung nach nur schwer mit einer übersichtlichen Fehlerbehandlung kombinieren. Danach werden ggf. Variablen angepasst ($I_delete_time) und dann wird das SQL-Query + Parameter an check_query übergeben.
 
Also nochmal zusammengefasst:
- Typerhaltung durch die Platzhalter (Integer kann nicht zu String werden, ausversehen)
- Maskierung von Sonderzeichen (mysql_real_escape_string)
- Neu und noch ausbaufähig: Validierung gegen Whitelist (bei anderen Zeichen, Abbruch)
 
Grüße

[Powie]

Ja soweit ist das klar.
Das die $pdb->query nichts filtert ist gewollt, die ist auch nicht zum Filtern gedacht!
Die check_query habe ich entsprechend integriert, diese macht extrem Sinn, die Geschichte mit der Whitelist hingegen halte ich für falsch. Diese ist fehl am Platz wenn man zum Beispiel HTML oder andere Dinge in die DB speichert. dafür müsste man die Whitelist soweit erweitern das sie schon gar keinen Sinn mehr macht, denn \" und \' und  gehören da einfach dazu!
Also $pdb->check_query() ist erstmal drin,
$pdb->query() bleibt wie gehabt da diese intern so auch weiterverwendet wird, was uns aber nicht davon abhalten könnte vielleicht eine
checked_query() zu definieren die beides zusammenführt!

[k00ni]

/uploads/emoticons/icon_e_smile.gif.f7ec63a2b1c3d90a9415e40455642502.gif\" alt=\":-)\" />  :H:
Ich hatte dies schon bedacht mit den Sonderzeichen, wie sie bspw. im BBC-Code vorkommen. Die Whitelist wird nur aufgerufen, wenn eine gewisse Variable gesetzt wurden.
 

 
nämlich $Bo_check_chars, wie oben schon geschrieben. Wäre auch dafür, dies nicht immer einzuschalten. Man könnte es per Parameter bestimmen oder wie derzeit bei mir gesetzt, per zentraler Variable. Wobei mir ersteres besser gefällt, da man flexibler ist.
Kann man eigentlich in bei der Validierung durch die regulären Ausdrücke maskierte Dinge umgehen? Also wenn der ein \" \\\' \" findet, dass er dann nicht meckert?
 
Grüße

[k00ni]

Was mir auch aufgefallen, dass man Prepared-Statements, wie sie in MySQL implementiert sind, nicht direkt umsetzen kann, da bspw. das System hier bei uns noch auf MySQL 4.0.x läuft und dies 4.1 benötigt. Viele andere User werden dies wahrscheninlich ähnlich haben oder sogar noch Version 3.x. Also fiele das eigentliche Prepared-Statement soweit flach.  :-/

[Powie]

und das sie wenn ich es richtig verstehe sowieso mysqli benötigen...

[mahe]

Original von Powie und das sie wenn ich es richtig verstehe sowieso mysqli benötigen...
[/quote] jop