nicht angemeldet
echo $begrüßung;
Die übergebenen Parameter für Datenbank- und Tabellennamen werden nicht maskiert, was man unter Umständen für SQL-Injection ausnutzen kann.
Hm, eine ernste Sache, die du da ansprichst - erst mal vorweg, denkst du wirklich jemand könnte auf die Idee kommen, die Export Klasse mit Userdaten zu füttern?
Dieses Argument habe ich kommen sehen :-) Es ist aber schlicht egal, an welcher Stelle du nicht damit rechnest, dass so etwas passieren könnte. Wenn es passiert, ist es zu spät und dein Argument, dass du niemals mit von außen manipulierten/manipulierbaren Daten rechnest ist dann auch kein Trost mehr. Es gibt ja auch noch den register_globals-Mechanismus, der ungefragt Variablen im Programm erzeugt. Gehe nicht davon aus, dass der Anwender deiner Klasse alles richtig macht.
Wie dem auch sein - das Script sollte natürlich so oder so sicher sein, allerdings dachte ich bis jetzt, dass es das ist. Jetzt könnte ich natürlich über die Datenbank- und Tabellennamen noch mal mysql_real_escape_string() drüber laufen lassen, jedoch sehe ich folgendes Problem darin:
Angenommen magic_quotes ist aktiviert, dann müsste man auf $_POST bzw. $_GET Daten erst stripslashes() laufen lassen, weil es sonst ein doppeltes Escapen geben würde - allerdings kann ich im Script ja nicht feststellen, ob die Werte aus $_POST bzw. $_GET kommen oder ob sie von Hand eingegeben wurden - es könnte also sein, dass ich mit stripslashes() was entferne, was gar nicht durch magic_quotes dahin gekomme ist.
Gut erkannt. Es ist aber nicht deine Aufgabe für die Richtigkeit der übergebenen Parameter zu sorgen. Du hast sie nur unbeschadet ans Ziel zu bringen oder mit Fehlermeldungen um dich zu werfen, wenn das nicht geht. Man muss nicht alle Dummheiten der Anwender korrigieren wollen, man muss nur dafür sorgen, dass sie keinen Schaden anrichten.
Weiterhin würde ein „wissender“ PHP Programmierer ja über die Daten selber noch mal mysql_real_escape_string() drüber laufen lassen, wozu es dann auch zu einem doppelten Escapen kommen würde.
Das ist nicht seine Aufgabe. Er kann auch nicht wissen, in welcher Form du die übergebenen Werte verwendest. Vielleicht brauchst du sie an drei Stellen deiner Klasse einmal so, einmal so und dann noch einmal ganz anders. Es wäre kontraproduktiv, sie bereits nur für einen einzigen Anwendungsfall hin zu optimieren. Wenn er das doch tut, bekommt er von dir entweder nicht das gewünschte Ergebnis oder eine Fehlermeldung, und zwar weil die Datenbank diesen sonderzeichenbehafteten Tabellen-/Datenbank-Namen nicht finden konnte, nicht weil du ihn irgendwie analysiert hast.
Ich predige ja auch gelegentlich, Demaskierungen ankommender Daten so früh wie möglich durchzuführen und abgehende Daten erst und nur zur Übergabe an das Zielmedium zu maskieren. Hin- und Hermaskierungen mitten in der Verarbeitung tragen nicht grade zur Übersichtlichkeit des Prozesses bei.
Und last but not least bin ich mir gar nicht sicher, ob man beim Datenbanknamen SQL Injections machen kann...
Alle Stellen zu denen unmaskierte Daten gelangen können sind anfällig. Aus deiner Sicht magst du alle (dir bekannten) Angriffsmöglichkeiten als nicht zielführend bewertet haben, und trotzdem hast du die eine Möglichkeit übersehen, bei der es passieren kann.
Auch bei den Tabellennamen weiß ich nicht so genau - erst mal setze ich die Namen alle zwischen Backticks (`) und zweitens findet das Script selber ja die Tabellennamen herraus indem es sie sich von MySQL holt.
Nun, man kann export_table_data() auch zu Fuß aufrufen. PHP4 sei Dank (oder auch nicht) sind ja alle Methoden public.
Wie dem auch sein - ich habe die in Sachen Error Handling überarbeitete Version jetzt mal online gestellt.
Ich würde mich freuen, wenn du sie dir nochmal anschaust.
var $newline = "\n";
Ich würde keine Klassenvariable dafür verwenden, zumal es außer einer Quelltextänderung keine Möglichkeit gibt, die zu verändern. Mein Vorschlag wäre eine Konstante zu definieren, falls sie noch nicht anderswo gesetzt wurde. So kann man vor dem Include deiner Klasse Einfluss darauf nehmen.
if (!defined('NL'))
define('NL, "\n");
O.K. Der Anwender kann die Eigenschaft nach erfolgter Instantiierung ändern. Dazu muss er aber voraussetzen, dass sie bis dahin noch nicht verwendet wurde. (Die Analyse des Konstruktor-Quelltexts, um das herauszufinden, ist keine sehr feine Option.)
Alternativ wäre auch noch ein weiter optionaler Konstruktor-Parameter denkbar.
Die Zwischenvariable $con in Zeile 77 ff. ist nicht nötig. Du kannst gleich $this->con verwenden.
Teilweise gibt es Ansätze, das Ergebnis variieren zu können (z.B. Parameter $drop_if_exists für Methode export_table_structure(), Zeile 126). Beim Aufruf der Methode (Zeile 251) wird dann aber ein fest vorgegebener Wert verwendet. Da gibt es also noch Erweiterungspotential für die Klasse. :-)
false (Zeilen 138 und 157) ist kein wirklich geeigneter Wert, um einen String zu erweitern (Zeile 251f.) Außerdem sagt make_dump() in dem Fall, dass alles in Ordnung wäre. Nur das Ergebnis ist dann nicht wie vom Anwender erwartet ...
(Mit sprintf() bekommst du Konstrukte wie in Zeile 196 ff. etwas lesbarer gestaltet.)
Das sind nur Dinge, die mir beim Drüberschauen aufgefallen sind. Die Klasse einmal anzuwenden habe ich nicht versucht.
echo "$verabschiedung $name";
P.S. Sind meine obigen Aussagen - "Vertraue nicht den vom User übergebenen Daten" vs. "Vertraue dem was der User übergibt" - widersprüchlich? Das scheint mir auf den ersten Blick auch so. Es kommt darauf an, die richtige Haltung bei diesem Spagat zu finden; Absicherung des eigenen Programmteils, so dass Fehler keine negativen Auswirkungen haben, und "Anwenderbevormundung", indem man seine Parameter kritisiert, in ein optimales Verhältnis zu setzen.