Formulardaten bereinigen

[Forumsgast]

Hallo zusammen,
was haltet ihr von der folgenden Methode Formulardaten zu verarbeiten?

Methode 1

# Formulareingaben filtern
function clean_postvars($var)
{
  if(!empty($var))
  {
    $var = trim($var);
    $var = strip_tags($var);
    $var = htmlspecialchars($var,ENT_QUOTES,'UTF-8');
    $var = stripslashes($var);
  }

  return $var;
}

# $_POST Array durchlaufen und überschreiben
foreach($_POST as $key => $value)
{
  $content = clean_postvars($value);
  $_POST[$key] = $content;
} 

Methode 2

# $_POST Array durchlaufen und in Variablen packen
foreach($_POST as $key => $value)
{
  $content = clean_postvars($value);
  ${$key} = $content;
} 

Wenn die übergebenen Post-Keys gegen eine Whitelist geprüft werden, spricht dann noch etwas gegen diese Vorgehensweise?

Ich finde es optimal, habe aber gelesen es würde register_globals nachbilden. Klingt auch logisch, zumindest bei der zweiten Variante. Was meint ihr dazu? Gibt es eventuell noch bessere Möglichkeiten?

[Forumsgast]

Das Umwandeln in die Entities mit htmlspecialchars ist an dieser Stelle falsch. Eine Usereingabe :

Mein Name ist "Klein"

wird so vor dem Speichern in Mysql verstümmelt. Richtig ist es, die Sonderzeichen abzuspeichern und diese erst vor dem Ausgeben im HTML-Context mit htmlspecialchars zu bearbeiten.

Ansonsten kannst du die beiden Codeteile auch zusammenfassen:

# $_POST Array durchlaufen und überschreiben 
foreach($_POST as $key => $var) 
{ 
    $var = trim($var); 
    $var = strip_tags($var); 
    $var = stripslashes($var); 
  $_POST[$key] = $var; 
} 

Der zweite Ansatz ist unnütz. man hat alles im superglobalen $_POST-Array.

[Forumsgast]

Hallo,

wenn du die formular daten in eine DB wegspeicherst würde ich dir zu
noch zu mysql_real_escape_string() raten.

ich verwende derzeit folgende function:

function makeSave($usereingabe){
     if(get_magic_quotes_gpc()) {
         $usereingabe = stripslashes($usereingabe);
     }
     $usereingabe= mysql_real_escape_string(trim($usereingabe));
     $usereingabe=strip_tags($usereingabe);
return $usereingabe;
} 

ob meine funktion alleine ausreicht weiss ich jedoch auch nicht sicher

[Forumsgast]

Ja, mysql_real_escape_string ist schon klar, ich arbeite übrigens mit mysqli. Das aber direkt mit einzubauen anstatt einzeln aufzurufen ist keine schlechte Idee.

Mir geht es bei meiner Frage hauptsächlich um die Sache mit dem Umschreiben des Post-Arrays um die gefilterten Daten so auch zur weiteren Verarbeitung im Script und Ausgabe auf dem Bildschirm sicher verwenden zu können.

[Forumsgast]

Dazu verwende ich normal noch ein Listen Array um nur gewollte $_POST zu verarbeiten. so ist die anzahl der verarbeiteten variabelen begrenzt.

$list = array('var1','var2','var3');
// teste alle Werte
foreach ($list as $value) {
    $_POST[$value]=makeSave($_POST[$value]); 

so bauste auch keine register globals nach

[Forumsgast]

Auch eine nette Variante. Danke für die Tipps!

Ich prüfe normal alle übergebenen Array-Keys vom Post-Array mit einer Whitelist.

if(isset($_POST['send']))
{
  $allowed = array('field1','field2','field3','send');
  $check = array_keys($_POST);

  if($check === $allowed)
  {
    require('process.php');
  }
  else
  {
    die('Ungülte Daten übertragen');
  }  
} 

So kommen erst gar keine ungewollten Post-Werte durch.

[Forumsgast]

Es ist ebenso unsinnig, die eingegebenen Daten erstmal komplett mit mysql_real_escape_string() zu bearbeiten. Das mach man erst im Querystring.

Wenn du z.B. ein Formular verifizierst und einen Eingabefehler feststellst, dann gibst du das Formular mit den schon vorhandenen Daten zusammen wieder aus, und dann hast du in den Eingabefeldern misshandelte Daten.

  $allowed = array('field1','field2','field3','send'); 
  $check = array_keys($_POST); 

Dieser Code scheitert bei nicht-angeklickten Checkboxen oder nicht angeklickten Select-Boxen, weil dann nichts übertragen wird.

[Forumsgast]

Ja, das ist absolut korrekt, die Erfahrung habe ich auch schon gemacht. Als Workaround habe ich in dem Fall wo es erforderlich war eben erst überprüft ob dieser Post-Key gesetzt ist.

Das mit mysql_real_escape_string habe ich bislang auch immer im query gemacht, dachte aber daran dass man da ebenso eine Schleife für machen könnte. Für mich persönlich ist es jedoch unsinnig da ich mit Prepared Statements arbeite.

Das Umwandeln in die Entities mit htmlspecialchars ist an dieser Stelle falsch. Eine Usereingabe :

Mein Name ist "Klein"

wird so vor dem Speichern in Mysql verstümmelt. Richtig ist es, die Sonderzeichen abzuspeichern und diese erst vor dem Ausgeben im HTML-Context mit htmlspecialchars zu bearbeiten.

Wie sieht es in dem Fall aus wenn man die übermittelten Daten dierekt im Formular wiedergeben möchte im Fehlerfall damit der Nutzer nicht alle Eingaben neu machen muss?

Habe da jetzt gegensätzliche Meinungen zu. Bislang dachte ich es wäre gut.

[Forumsgast]

Faustregel:

- Vor Speichern in DB - mysql_real_escape_string (ausser bei. P.S.)
- Vor Ausgabe als HTML - htmlentities

Mehr nicht.
Allenfalls generell einmal strip_slashes auf alle Parameterdaten, wenn magic_quotes an ist.

[Forumsgast]

Das mit htmlentities leuchtet mir ja noch halbwegs ein, aber warum hälst du es für unnötig Tags und Leerstellen zu entfernen?

Ich persönlich finde es sinnvoll und möchte verhindern das Schadcode von meinem Script ausgeführt wird. Darum die ganze Filterung, insbesondere die Prüfung von übertragenen Daten ob diese so in einen vorgegebenen Rahmen passen.

[Forumsgast]

Tags werden durch htmlentities maskiert. Ergo - keine Gefahr. Ob das jetzt erwünschter Inhalt ist, interessiert an dieser Stelle nicht. Denn das ist Aufgabe der Eingabevalidierung, die vor jeder Ausgabe/Speicherung stattzufinden hat. In einem Webdesign-Kommentar können Tags sehr wohl zum Inhalt gehören. Eine <Email> muß auch nicht weggeschnitten werden, nur weil sie wie html aussieht.
Dasselbe gilt für Leerzeichen - pauschal würde ich die nicht wegschneiden. Nur wenns unerwünschter Inhalt ist.

[Forumsgast]

Ich habe den Abend genutzt um mal ausgiebig zu experimentieren. So habe ich mal verschiedene Angriffsmöglichkeiten ausprobiert und bin zu folgendem Schluss gekommen:

1. Alle Benutzereingaben und dynamisch eingebundene Inhalte müssen explizit geprüft werden.

2. Arbeitet man mit Prepared Statements wird der übertragene Inhalt so gespeichert wie er gesendet wurde. Ein Injection-Code steht also sauber in der Datenbank und wird nicht von MySQL interpretiert. Da das Query nicht abgefälscht werden kann ist es zwar möglich dass Müll in der Datenbank steht, aber eine SQL-Injection ist eher unwahrscheinlich.

3. Arbeitet man mit herkömmlichen Mysql Queries sollte man tunlichst von mysql_real_escape_string() gebrauch machen. Ansonsten ist ein Abfälschen des Queries möglich.

4. Was ich bislang für gut hielt ist etwas übertrieben. Sofern jede Benutzereingabe auf einen gültigen Wert geprüft wird ist es schwierig Schadcode zu übermitteln. Man kann je nach Bedarf und persönlicher vorliebe auch auf trim und striptags verzichten. Das ist wohl eher eine Geschmacksfrage.

5. htmlentities ist sinnvoller alls htmlspecialchars und die Benutzung ist erst vor Ausgabe von dynamisch erzeugten Inhalten oder Benutzereingaben notwendig. Was in der Datenbank steht ist da eher zweitrangig. Das würde ich jedoch einheitlich mit einer Funktion regeln. Der Vorteil von Funktionen sollte allgemein bekannt sein.

Ich werde nach diesen Erkenntnissen doch ein wenig anders programmieren.

Das $_Post-Array durchlaufen zum Beispiel kann aber dennoch nützlich sein wenn man zum Beispiel Usereingaben direkt wieder ausgeben möchte.

[Forumsgast]

Das hätte ich Dir aber auch sagen können

Nee, mach mal ne schöne Liste fertig. Vielleicht wirds ja mal was fürs Wiki (Anfänge existieren ja bereits). Bitte ruhig hier noch Erkenntnisse veröffentlichen.

zu 5/

notwendig

und sinnvoll! In einer Datenbank sind Plaininhalte bspw. besser zu durchsuchen, als Ausgabe in einem Plaintextfile (eg. Logfile) besser zu lesen und ohnehin ungefährlich...
htmlentities ist sinnvoller, ABER NUR, wenn man konsequent UTF-8 als Script/DB/Formular Zeichensatzstandard verwendet.