Aus dem Rechenzentrum

Wenn es um Sicherheitsprobleme geht, werfen die entsprechenden Security-Advisories gerne mit Begriffen wie „cross-site scripting“, „cross-site request forgery“ oder „sql injection“ um sich. Aus aktuellem Anlass können wir mal ein praktisches Beispiel einer real existierenden Software nehmen (deren Name lieber ungenannt bleibt), mit der man mandantenfähig Rechnungen erstellen kann, die sich ein Kunde auf seinem Webspace installiert hat. Sie lief anfangs nicht, und die Basis seiner Anfrage war dann eher erstmal der Wunsch nach „Könnt ihr das zum Laufen bringen“-Support. Nachdem ich mir dazu kurz einige der fraglichen PHP-Dateien angeschaut hatte, wurde daraus dann aber eher ein kleines Lehrstück, das ich hier gerne (bis auf die Usernamen) unverändert weitergebe.

[...]

Es sieht mir auf den ersten Blick so aus, als könne man da sehr viel
verbessern, wenn ich das mal vorsichtig so sagen darf. ;-)  Ich sehe
schon auf den ersten Blick Angriffspunkte für SQL Injection sowie eine
Abhängigkeit von register_globals = On. Das erste ist ein handfester
Fehler; das zweite eröffnet eventuelle Angriffspunkte, die nicht sein
müssten. Freundlich gesagt sieht man der Software an, das sie von 2006
ist. :)

Willst du mal sehen, wie das mit SQL Injection funktioniert?

Gib mal bei der Anmeldung als Benutzername deinen Zugang "max" ein
und dann als Passwort dieses hier:

        '||ID='1

Schwupps, bist du drin. Das klappt mit "moritz" genauso. Und mit jedem
anderen auch.

Hintergrund ist, dass in der mod/authent.php ausgeführt wird:

        $result=$dbverbindung->run_sql("SELECT ID,User,Pwd FROM Admin WHERE User='"
          .$user."' AND Pwd='".$pwd."'");

Das heißt, wenn du "max" und "geheim" angibst, steht da:

        SELECT ID,User,Pwd FROM Admin WHERE User='max' AND Pwd='geheim'

Genauso steht dann aber auch da, wenn du mein Trick-Passwort anwendest:

        SELECT ID,User,Pwd FROM Admin WHERE User='max' AND Pwd=''||ID='1'

Also "selektiere alle User, deren Name 'max' ist und bei denen das
Passwort leer ist (was nie vorkommt), oder alle User, deren ID = 1 ist".

Das "Schöne" ist: Es wird so einfach der User 1 selektiert. Dass du da
"max" angibst, tut überhaupt nichts zur Sache. Siehe hier:

        mysql> SELECT ID,User,Pwd FROM Admin WHERE User='moritz' AND Pwd=''||ID='1';
        +----+--------+---------+
        | ID | User   | Pwd     |
        +----+--------+---------+
        |  1 | max    | geheim  | 
        +----+--------+---------+
        1 row in set (0.00 sec)
        
Aber: Da mod/authent.php immer den Namen aus dem _Formular_ in die
Session als angemeldeten User übernimmt, musst du nicht mal die numerische
ID kennen. Es reicht, dass das SQL-Statement _irgendeinen_ Datensatz
zurückliefert; dann schreibt es den Namen aus dem Formular in die
Session. Sprich, beim vorigen SQL-Statement wäre ich dann als "moritz"
angemeldet, obwohl das SQL-Statement deinen Account geliefert hat. Ich
brauche also nur einen beliebigen Benutzernamen sowie '||ID='1, dann
komme ich rein.

Richtig WÄRE im Programmcode übrigens:

        $result=$dbverbindung->run_sql("SELECT ID,User,Pwd FROM Admin WHERE User='"
          .mysql_real_escape_string($user)."' AND Pwd='".mysql_real_escape_string($pwd)."'");

Allerdings müsste man solche Fehler an ...

        $ grep -r ">run_sql" . | wc -l
        92

... Stellen in Ordnung bringen. Da kann man's auch gleich selber
schreiben. (Dass das Problem nicht nur beim Login vorliegt, sondern bei
jedem SQL-Statement, zeigt mir auch, dass das kein "Bug" ist, den der
Autor dort versehentlich eingebaut hat, sondern dass ihm grundlegendes
Verständnis vom Design von Webapplikationen fehlt. Ist ja nicht so, dass
die PHP-Dokumentation das nicht alles haarklein erklären würde..! Da
findet sich Code wie der von ihm fast 1:1 in der Doku - als Beispiel
dafür, wie man's NICHT macht.)

[...]

Folge: Der Kunde verpackt das installierte PHP-Script noch hinter einer .htaccess-Datei zum „übergeordneten“ Passwortschutz. Da es sich um eine Anwendung handelt, die er ausschließlich selbst einsetzt und die keinen Dritten zur Verfügung stehen wird, ist dies hier ein – vom Sicherheitsaspekt her – akzeptabler Kompromiss. Dass man üblicherweise lieber Software von Leuten einsetzen sollte, die ihr Handwerk verstehen, versteht sich hoffentlich von selbst.

Da ich mittlerweile verstärkt mit Catalyst als Framework arbeite und hier als Model-Integration typischerweise DBIx::Class verwendet wird, komme ich nicht umhin, mich weitergehend mit der entsprechenden Syntax auseinanderzusetzen. Dabei mache ich fast jeden Tag die gleiche Erfahrung: DBIx::Class macht die einfachen Anwendungsfälle von SQL noch einfacher, aber die schwierigeren Anwendungsfälle noch viel schwieriger, denn die Klimmzüge, die hier bisweilen in der kruden SQL::Abstract-Syntax zu vollziehen sind, haben es wirklich in sich.

Als Beispiel erstmal der einfache Fall:

my $contact = $schema->resultset('Contact')->search(
  {
    'n_family' => 'Pasche',
    'n_given' => 'Jonas',
    'addresses.city' => 'Mainz'
  },
  { join => 'addresses' }
)->first;

Das ist ja noch durchaus zu überblicken – insbesondere das Zusammenführen mit der Relation ‚addresses‘ ist einfach und elegant.

Will ich nun allerdings die Levenshtein-Distanz mit einbauen, um schreibfehler-tolerant zu suchen, ergeben sich gleich zwei Problemfelder:

1. Ich brauche eine MySQL-Funktion auf der linken statt auf der rechten Seite des Vergleichsoperators. Okay, prima dokumentiert im DBIx::Class::Manual::Cookbook.
2. Sowohl auf der linken als auch auf der rechten Seite des Vergleichsoperators sollen Platzhalter (bind values) zum Einsatz kommen: Links der String, mit dem ich vergleichen will; rechts die maximale Distanz, die ich natürlich gerne flexibel als Variable einsetzen möchte. Auch hier hilft noch das Cookbook, wechselt hierbei aber im Beispiel klammheimlich zu literal SQL – gut daran zu erkennen, dass die Suchkriterien nun plötzlich kein Hashref mehr sind. Okay, habe ich nachvollzogen.
3. Dummerweise möchte ich aber nicht nur einen Wert vergleichen, sondern gleich mehrere. Dabei kann ich das Cookbook-Beispiel natürlich ausbauen und einen entsprechend langen SQL-String selbst schreiben und dann meine ganzen bind values angeben. Das ist jedoch, gelinde gesagt, unglaublich hässlich und vor allem schwer zu warten, weil die bind values innerhalb von Perl an einer ganz anderen Stelle stehen als das SQL-Statement. Möchte ich zum Beispiel ein Feld mittendrin entfernen, muss ich akribisch meine bind values durchzählen, damit ich dort den zugehörigen Wert entferne. So dann bitte lieber nicht.

Nun kann ich auch beim Hashref für die Suchkriterien bleiben, und das funktioniert auch weiterhin, wenn ich SQL-Strings als Hashkeys angebe. Nur wie ich dann auf der rechten Seite, also bei den Hashwerten, die Suchkriterien angeben kann, das hat sich mir einfach nicht erschlossen. Nach längerer Lektüre bleibt aus meiner Sicht nur die Möglichkeit, auch auf der rechten Seite handgeschriebenes SQL zu verwenden, und dort dann die bind values für beide Seiten des Vergleichs anzugeben.

Und so geht’s: Ich gebe den rechten Teil des Vergleichs als doppeltes Arrayref an – die abstrahierte Schreibweise für „Hier kommt jetzt literales SQL mit bind values„. Dabei ist der erste Wert der literale SQL-String; danach folgen die zwei bind values – erst der für links (Vergleichsstring), dann der für rechts (maximale Levenshtein-Distanz) – und da laut Cookbook DBIx::Class hierbei bindtype=columns in SQL::Abstract setzt, muss ich hierbei die etwas krude Syntax der Wertzuweisung über Dummy-Spaltennamen (in meinem Fall einfach „d“) benutzen, wie die oben verlinkte Dokumentation bereits erklärt.

Und so sieht’s final aus:

my $dist = 2;
my $contact = $schema->resultset('Contact')->search(
  {
    'LEVENSHTEIN(n_family, ?)'       => \[ '<= ?', [ d => 'Paschke' ], [ d => $dist ] ],
    'LEVENSHTEIN(n_given, ?)'        => \[ '<= ?', [ d => 'Ionas' ],   [ d => $dist ] ],
    'LEVENSHTEIN(addresses.city, ?)' => \[ '<= ?', [ d => 'Meins' ],   [ d => $dist ] ]
  },
  { join => 'addresses' }
)->first;

Ich bin nicht ganz sicher, ob das wirklich der optimale Weg ist – es erscheint mir ein wenig unsauber, auf der rechten Seite einen bind value mit hineinzumogeln, der eigentlich nach links gehört. Aber ich habe keine sauberere Möglichkeit dafür gefunden – also nehme ich es jetzt mal so hin.

Und so sieht das dann in SQL aus (Ausgabe gekürzt und der besseren Lesbarkeit wegen mit Umbrüchen und Einrückungen versehen; hinter dem Doppelpunkt folgen dann die bind values):

SELECT me.id, [...]
  FROM contact me
       LEFT JOIN address addresses ON addresses.contact_id = me.id
 WHERE (
         (
           LEVENSHTEIN(n_family, ?) <= ?
           AND
           LEVENSHTEIN(n_given, ?) <= ?
           AND
           LEVENSHTEIN(addresses.city, ?) <= ?
         )
       ): 'Paschke', '2', 'Ionas', '2', 'Meins', '2'

Fraglich ist für mich noch, wieso SQL::Abstract das WHERE-Kriterium gleich in doppelte Klammern setzt, wo doch eigentlich nicht einmal einfache vonnöten wären, aber das tut der korrekten Funktion keinen Abbruch.

Hat mich jetzt auch nur zwei Stunden gekostet – für ein SQL-Statement, das ich per Hand aus dem Kopf innerhalb von zwei Minuten fertiggebracht hätte. Wie gesagt: Kompliziertes wird mit SQL::Abstract noch komplizierter. Aber vielleicht ist das ja auch nur dem Umstand geschuldet, dass sich die für SQL::Abstract-Syntax notwendigen Synapsen bei mir noch nicht gebildet haben. Feedback willkommen!