PHP-Seite absichern

[Gast]

Hallo erstmal,

ich schreibe gerade mein eigenes kleines CMS, da die CMS von der Stange für mein Vorhaben zu überladen sind. Das ganze soll ja auch halbwegs sicher sein, jetzt frage ich mich, was man alles für die Sicherheit tun muss. Bisher habe ich folgende Punkte umgesetzt:

- Wenn etwas includet wird (a la index.php?site=news) wird $site immer mit file_exists geprüft

- wenn Benutzereingaben in einem Formular vorkommen, dann werden sie mit $_POST empfangen und htmlspecialchars() darauf angewandt.

- Sachen aus der URL werden mit $_GET geholt und ebenfalls htmlspecialchars() angewandt

- Passwörter werden als salted hash gespeichert

Viel mehr ist es leider nicht, aber ich frage mich, was da sonst noch kommen soll. Usereingaben werden unschädlich gemacht, genauso wie Sachen aus der URL oder falsche includes. Das sind ja eigentlich die 3 Möglichkeiten, die ein Angreifer als Türe nehmen kann, oder? Dateiuploads gibt es nicht, Cookies werden auch keine gesetzt.

Gibt es denn noch andere Tore, wo eingewirkt werden kann?

Das mit dem XSS hab ich noch nicht so kapiert. Soweit ich das verstanden habe, soll auf Seiten, die dynamisch erzeugt werden, schädlicher Code ausgeführt werden können. Dieser wird eingeschleust vom Angreifer, der sich zu nutze macht, dass die Seite eben dynamisch erzeugt wird und nicht statisch ist. Das Opfer ruft die vom Angreifer erzeugte Seite auf und es macht peng.Nur kann ich mir das überhaupt nicht vorstellen... wie soll denn der Schadcode da reinkommen?
Wie gesagt, auf Post und Get wende ich htmlspecialchars an, Dateiuploads und Cookies spielen erstmal keine Rolle.

Danke schonmal :)

[Gast]

Achja, worauf ich eigentlich auch noch hinauswollte: SQL-Injections, XSS usw. beschreiben ja eine große Vielfalt auf Sicherheitslücken. Ist der Ursprung von dem ganzen Kram ,wie er auch heißen mag, nicht schlicht und einfach das zulassen "bösartiger" Usereingaben, egal ob per Textfeld, Cookie oder URL?
Wenn man diese gründlich prüft (z.b. mittels Regex+htmlspecialchars usw.), könnte man dann nicht alle Sicherheitsprobleme auf einmal erschlagen?

[samm]

Neben htmlspecialcharacters gibt's noch strip_tags und, wenn eine Datenbank im Einsatz ist, <DB>->escape_string.
Ja, das unüberprüfte Übernehmen von Benutzereingaben ist normalerweise das grösste Risiko. Ansonsten gilt es v.a. die Verzeichnisse, worin die php-Files liegen, und die Datenbank - sofern vorhanden - so gut wie möglich zu schützen. Das heisst: Den Webserver so sicher wie möglich konfigurieren, Dateisystem so sicher wie möglich konfigurieren, PHP so sicher wie möglich konfigurieren (z.B. alle Befehle ausser den verwendeten verbieten). Sollte ein Angreifer doch einmal an die PHP-Files kommen, stelle sicher, dass die darin erwähnten DB-Verbindungen einen Benutzer verwenden, der nur gerade die Rechte hat, die er wirklich benötigt. Stelle ausserdem sicher, dass du eine Möglichkeit hast, die Website zuverlässig vom Netz zu trennen, alle Löcher zu finden, und erst wieder online zu stellen, wenn jedes Loch gestopft wurde - nicht einfach nur ein Backup laden.

[RMC]

Alles was bösartige Benutzereingaben für Datenbankabfragen betrifft: Prepared Statements verwenden ;) Da werden alle Eingaben nur mehr als "value" interpretiert.

[darph]

- Wenn etwas includet wird (a la index.php?site=news) wird $site immer mit file_exists geprüft

Die Dateien, auf die ein Angreifer zugreifen wollte, die existieren.

Wäre es nicht sinnvoller, die Variable als "slug" herzunehmen? Als Feld in einer Datenbank, zu der dann ein Eintrag oder ein Pfad etc. gespeichert wird. So wird auch wirklich nur das angezeigt, was auch dort gespeichert wird. Alles andere gibt einen Fehler.

Egal mit wie vielen Mechanismen das absichert, von einer von Benutzer angegebenen Variable direkt auf das Dateisystem zu zielen, halte ich persönlich für … uhm … fahrlässig.

[rotalever]

Alles was bösartige Benutzereingaben für Datenbankabfragen betrifft: Prepared Statements verwenden ;) Da werden alle Eingaben nur mehr als "value" interpretiert.
Für manche Datenbanken gibt es auch parameterisierte Statements, die das gleiche bewirken aber wo man nicht den Umgang über prepared Statements nehmen muss.

Eingaben des Benutzers sollten am besten mit Whitelists geprüft werden (Regex).

[Gast]

Hey, danke mal für die Anworten :)


Das heisst: Den Webserver so sicher wie möglich konfigurieren, Dateisystem so sicher wie möglich konfigurieren, PHP so sicher wie möglich konfigurieren (z.B. alle Befehle ausser den verwendeten verbieten). Sollte ein Angreifer doch einmal an die PHP-Files kommen, stelle sicher, dass die darin erwähnten DB-Verbindungen einen Benutzer verwenden, der nur gerade die Rechte hat, die er wirklich benötigt.

Es handelt sich um Webspace (vom relativ bekannten all-inkl.com), ich weiß nicht, wie tief ich da konfigurieren darf, mal sehen...
Aber: register_globals, magic quotes und directory listing habe ich bis jetzt deaktiviert.


Alles was bösartige Benutzereingaben für Datenbankabfragen betrifft: Prepared Statements verwenden ;) Da werden alle Eingaben nur mehr als "value" interpretiert.

Jap, habs mir mal angesehen und sieht gut aus :)


Die Dateien, auf die ein Angreifer zugreifen wollte, die existieren.
Wäre es nicht sinnvoller, die Variable als "slug" herzunehmen? Als Feld in einer Datenbank, zu der dann ein Eintrag oder ein Pfad etc. gespeichert wird. So wird auch wirklich nur das angezeigt, was auch dort gespeichert wird. Alles andere gibt einen Fehler.
Egal mit wie vielen Mechanismen das absichert, von einer von Benutzer angegebenen Variable direkt auf das Dateisystem zu zielen, halte ich persönlich für … uhm … fahrlässig.

Also meine Intention ist es ja, zu verhindern dass externe Dateien includet werden können.

Verstehe ich dich richtig, dass du meinst, der Angreifer kann damit zwar nichts fremdes includen, jedoch dafür auf alle anderen Dateien auf meinem Server außer den dafür vorgesehenen zugreifen?

Mein Code sieht bisher so aus (Auszug):



if (!preg_match("#^[A-Za-Z0-9]$#", $_GET['page'])) {
die();
} else {
$site = 'includes/' . $_GET['page'] . '.php';
if (file_exists($site))
{
include($site);
}
}


Die gewünschte Seite wird also aus der Url geholt und in einen vorgegeben Pfad reingepresst. Es muss also eine .php Datei sein und außerdem im includes-Ordner Stecken, wo nur die Dateien gelagert werden für die Öffentlichkeit. Zusätzlich könnte man 'page' mittels regex ja noch prüfen, so dass nur "A-Za-z0-9" erlaubt sind (so dass man keine Slashes zum Verzeichnissprung reinmachen kann).

Meinst du wirklich, dass ist fahrlässig? Werde gerne eines besseren belehrt, aber hätte das für relativ sicher gehalten.


Für manche Datenbanken gibt es auch parameterisierte Statements, die das gleiche bewirken aber wo man nicht den Umgang über prepared Statements nehmen muss.

Sind diese prepared statements nicht "state of the art" sozusagen? Würde die schon benutzen, wenn das keine großen Nachteile mit sich bringt.


Eingaben des Benutzers sollten am besten mit Whitelists geprüft werden (Regex).

Wird sowieso gemacht :)

[Sephiroth]

Verstehe ich dich richtig, dass du meinst, der Angreifer kann damit zwar nichts fremdes includen, jedoch dafür auf alle anderen Dateien auf meinem Server außer den dafür vorgesehenen zugreifen?

Meinst du wirklich, dass ist fahrlässig? Werde gerne eines besseren belehrt, aber hätte das für relativ sicher gehalten.

Er meint das somit auch wirklich nur Dateien eingebunden werden können, die dafür vorgesehen sind, egal ob nur in einem festen Verzeichnis oder nicht.
Beispielweise wo man blind "include $_GET['foo']" verwendet aber auch bei deiner Variante, wenn mit Prüfung des Names in dem Verzeichnis eine Datei liegt, die eigentlich nicht eingebunden werden soll.


Dein regulärer Ausdruck ist übrigens falsch: a-Z ist kein gültiger Bereich, da das kleine a nach dem großen Z im ASCII-Code steht (A-z ginge aber), und dein gültiger String darf laut regex nur eines der Zeichen sein. ;)
-> '/^[a-z0-9]$/i'

[Kinman]

Was imho noch wichtig ist:
Den direkten Zugriff auf zu inkludierende Daten verwehren.

mfg Kinman

[Gast]

Er meint das somit auch wirklich nur Dateien eingebunden werden können, die dafür vorgesehen sind, egal ob nur in einem festen Verzeichnis oder nicht.
Beispielweise wo man blind "include $_GET['foo']" verwendet aber auch bei deiner Variante, wenn mit Prüfung des Names in dem Verzeichnis eine Datei liegt, die eigentlich nicht eingebunden werden soll.


Dein regulärer Ausdruck ist übrigens falsch: a-Z ist kein gültiger Bereich, da das kleine a nach dem großen Z im ASCII-Code steht (A-z ginge aber), und dein gültiger String darf laut regex nur eines der Zeichen sein. ;)
-> '/^[a-z0-9]$/i'

Es war nur etwas besserer Pseudocode ;)
(Wie man im Text lesen kannm wollte ich das mit dem Regex in meinem geposteten code ja noch garnich umsetzen, habs dann aber doch schnell hingeschrieben. Im Text schrieb ich ja A-Za-z0-9).

Zum ersten Teil: dafür gibt es den includes-Ordner ja extra, genau dafür hatte ich ihn geplant. Ersetze "includes" durch "public" :D

[Gast]

@Kinman, wird wie folgt gemacht:

in index.php wird $include auf true gesetzt, und in den zu inkludierenden Dateien wird dann geprüft

if(isset($include) && $include==true) // dann fortfahren.

Hast du ne bessere Idee?

[Kinman]

Ich machs auch so ;)
Zusätzlich kann man sämtliche Verzeichnisse, welche nur Dateien beinhalten, die nicht direkt aufgerufen werden, mit htaccess schützen.

[The_Invisible]

Ich machs auch so ;)
Zusätzlich kann man sämtliche Verzeichnisse, welche nur Dateien beinhalten, die nicht direkt aufgerufen werden, mit htaccess schützen.

die geb ich erst gar nicht im document-root baum rein, somit stellt sich die frage gar nicht. ;)

mfg

[rotalever]

Sind diese prepared statements nicht "state of the art" sozusagen? Würde die schon benutzen, wenn das keine großen Nachteile mit sich bringt.
Was heißt state-of-the-art in diesem Fall? Eigentlich sind prepared Statements dazu gedacht Queries (minimal) zu beschleunigen, da der Query-Syntax nur einmalig von der DB-Analysiert werden muss und dann einfach mit Parametern gefüllt wird. Das bringt allerdings nur etwas Geschwindigkeit, wenn man die gleiche Query oft benutzt, ansonsten noch Overhead.

Auf Grund der Parameterübergabe haben prepared Statements halt noch den netten Nebeneffekt, dass keine SQL-injections möglich sind. Wenn allerdings normale parametrisierte Statements verfügbar sind, dann kann man auch die verwenden, wenn man das ganze prepared-zeugs nicht braucht. Es kann natürlich sein, dass MySQL einfache parametrisierte Statements nicht unterstützt (wie so vieles..) und man deshalb immer prepared Statements nimmt. In PostgreSQL gibt es jedenfalls einfache parametrisierte.

[cbs_66]

Kann jemand paar beispiel-Statements posten, mit denen man testen kann, ob ein SQL Injection auf der eigenen Seite funktionieren kann? Also was man zB in ein Formular eingeben könnte..
danke

[Kinman]

die geb ich erst gar nicht im document-root baum rein, somit stellt sich die frage gar nicht. ;)

mfg

Diese Möglichkeit besteht leider nicht immer, ist sonst natürlich der noch bessere Weg.

[rotalever]

Kann jemand paar beispiel-Statements posten, mit denen man testen kann, ob ein SQL Injection auf der eigenen Seite funktionieren kann? Also was man zB in ein Formular eingeben könnte..
danke
Injections werden meist durch so etwas verursacht:

example.com/index.php?username=foobar

und im Code

... "SELECT * FROM users WHERE username=\'".$username."\';";...


Wenn man jetzt foobar durch irgendwas ersetzt wie

foobar\'; Anderes SQL statement;\'

oder so in der Art. Man muss halt die Parameter durchgucken. Wenn man ausschließlich prepared statements oder parametrisierte statements verwendet ist man in jeden Fall auf der richtigen Seite.

[Gast]

Du könntest dein CMS nach Python portieren.
Python ist deutlich sicherer als PHP.

[darph]

Injections werden meist durch so etwas verursacht:

example.com/index.php?username=foobar

und im Code

... "SELECT * FROM users WHERE username=\'".$username."\';";...


Wenn man jetzt foobar durch irgendwas ersetzt wie

foobar\'; Anderes SQL statement;\'

oder so in der Art. Man muss halt die Parameter durchgucken. Wenn man ausschließlich prepared statements oder parametrisierte statements verwendet ist man in jeden Fall auf der richtigen Seite.Das stimmt so nicht ganz. PHP setzt immer nur eine SQL-Query auf einmal ab.


Meine Vorgehensweise ist so: Kein Programmteil darf selbst auf die Datenbank zugreifen. Dafür ist ein eigenes Datenbankmodul zuständig, welches Methoden zum Einfügen und Auslesen von Daten bereithält.

/**
* Fetches a user from the database identified by the id given.
*
* @param int $id The user's id
* @return User An object that represents the user and his metadata such as
* name and email-address.
*/
public function getUser($id) {
$query = sprintf("SELECT * FROM user WHERE id = %d",
mysql_real_escape_string($id));
$result = self::query($query);

if ($row = mysql_fetch_assoc($result)) {
$user = new User($row["id"]);
$user->setName($row["name"]);
$user->setEmail($row["email"]);
return $user;
} else {
return null;
}
}

// oder

/**
* Authenticates the user against his password. Returns an user-object
* containing his metadata.
*
* @param string $username the login-name of the user
* @param string $password the user's password
* @return User An object that represents the user and his metadata such as
* name and email-address.
*/
public function authenticate($username, $password) {
$query = sprintf("SELECT * FROM user WHERE name = '%s'"
." AND password = MD5('%s');",
mysql_real_escape_string($username),
mysql_real_escape_string($password));
$result = self::query($query);

if (mysql_num_rows($result) != 1) {

throw new DatabaseException("query for user ".$username.": "
.mysql_num_rows($result)." rows returned - but 1 expected",
DatabaseException::UNEXPECTED_ROW_COUNT);
} else {
$row = mysql_fetch_assoc($result);
$user = new User($row["id"]);
$user->setName($row["name"]);
$user->setEmail($row["email"]);
$user->activate(($row["active"] == 1));

Logger::instance()->notice(__METHOD__, "User ".$user->getName()." fetched from DB");
return $user;
}
}

// und in der Oberklasse:
protected function query($query) {
if (!($result = mysql_query($query))) {
//Errorhandling à la echo(mysql_error());
}
return $result;
}

[rotalever]

Wie soll das denn dann überhaupt mit den SQL-Injections funktionieren, wenn immer nur ein Statement ausgeführt wird?

Andere Sache: Kontaktformulare sind auch immer eine beliebte Sicherheitslücke. Wenn die nicht vernünftig abgesichert sind, wird dein Server bald zum Spam-Server.

[Birdman]

Wie soll das denn dann überhaupt mit den SQL-Injections funktionieren, wenn immer nur ein Statement ausgeführt wird?

Hmm? du kannst locker mehrere Statements mitgeben - diese dann einfach mit einem ";" trennen.
Das ganze geht wunderbar und wird auch so gemacht.

[The_Invisible]

Diese Möglichkeit besteht leider nicht immer, ist sonst natürlich der noch bessere Weg.

ja, wir bieten unseren kunden dafür ein eigenes verzeichnis, leider checken es die meisten gar nicht obwohl darauf hingewiesen wird.

btw
python ist genauso anfällig für exploits wie php und ruby. die sache ist nur das kaum ein webhoster python-support anbietet, dafür sind 99% der angebote mit php-only support. und wenn der programmier schund schreibt oder/und der webhoster seine php version nicht aktuell halten kann, kann die sprache nichts dafür. wenn ich sehe mit welcher webserver und php version manche seiten laufen wird mir ganz schlecht.

mfg

[rotalever]

Ich verstehe auch nicht warum immer alle auf PHP rumhacken. Wenn man sich mal anschaut, was für große erfolgreiche Projekte damit schon geschrieben wurden. Ich habe auch schon von Fällen gehört, wo man fest davon überzeugt war auf z.B. Ruby umzusteigen und nach zwei Jahren dann das ganze doch wieder mit PHP geschrieben hat, weil es irgendwie besser war.
So Sachen wie z.B. Ruby werden meiner Meinung nach viel zu sehr gehyped.

[darph]

Hmm? du kannst locker mehrere Statements mitgeben - diese dann einfach mit einem ";" trennen.
Das ganze geht wunderbar und wird auch so gemacht.
Ich hab's nicht getestet, aber: http://tut.php-quake.net/mysql-query.html#u3

[Sephiroth]

Ich hab's nicht getestet, aber: http://tut.php-quake.net/mysql-query.html#u3
Da haben sie auch und es steht ja auch bei PHP so (http://de.php.net/manual/en/function.mysql-query.php).
Grund: mysql_query ruft die MySQL-Funktion mysql_real_query (http://dev.mysql.com/doc/refman/5.0/en/c-api-multiple-queries.html) auf, und dort heißt es:
By default, mysql_query() and mysql_real_query() interpret their statement string argument as a single statement to be executed, and you process the result according to whether the statement produces a result set (a set of rows, as for SELECT) or an affected-rows count (as for INSERT, UPDATE, and so forth).

MySQL 5.0 also supports the execution of a string containing multiple statements separated by semicolon (“;”) characters. This capability is enabled by special options that are specified either when you connect to the server with mysql_real_connect() or after connecting by calling` mysql_set_server_option() (http://dev.mysql.com/doc/refman/5.0/en/mysql-set-server-option.html).

Die Möglichkeit beim connect das Flag CLIENT_MULTI_RESULTS zu setzen wird von PHP nicht angeboten (http://de.php.net/manual/en/mysql.constants.php) und die MySQL-Funktion mysql_set_server_option() gibt es auch nicht. Bei MySQLi muss man die Funktion mysqli_multi_query() verwenden (http://de.php.net/manual/en/mysqli.real-connect.php).
PDO verwendet kein MySQLi, also kann man es da auch nicht nutzen.

Aber es gibt ja nicht nur diese Form der SQL-Injection, sondern auch solche wo die WHERE-Klausel erweitert wird.
Normal: SELECT * FROM customers WHERE username = 'timmy'
Injection: SELECT * FROM customers WHERE username = '' OR 1''

Oder wo beim Insert mehr eingefügt wird als es soll.
Normal: INSERT INTO benutzer (admin,benutzername,passwort) VALUES (0,'benutzername','passwort')
Injection: INSERT INTO benutzer (admin,benutzername,passwort) VALUES (0,'Benutzername','passwort'),(1,'Neuer_admin_account','passwort')

uvm.