Frage von Dereta, 68

PHP Include Methode so abgesichert gegen Angriff?

Hey leute,

benutze folgenden Code auf meiner Website um Seiten Dynamisch zu laden. Nun wollte ich fragen ob diese Methode so sicher ist oder ob es noch schwachstellen gibt.

Vielen dank für eure Antworten!

Mit freundlichen Grüßen Benjamin

Methode:

switch($_GET["site"])
{
    case "":
        include("./include/news.php");
        break;
    default:
        $file = preg_replace("/[^a-zA-Z]/i", "", $_GET["site"]);
        if ($file != $_GET["site"])
            include("./include/hacking.php");
        else
        {
            if (file_exists("./include/" . $file . ".php"))
            {
                include("./include/" . $file . ".php");
            }
            else
            {
                include("./include/error.php");
            }
            break;
        }
    break;  
}
Hilfreichste Antwort - ausgezeichnet vom Fragesteller
von RakonDark, 21

du solltest ein array nehmen, wo die gültigen seiten eingetragen sind und nur dann die gefundenen aufrufen . am besten noch mit einer rechte verwaltung, wer darf welche seiten nutzen .

niemals und nicht ein get verarbeiten , egal wieviel escapes du da reinmachst .


Kommentar von Dereta ,

Das wäre eine sehr gute Lösung! :) Und werde ich wohl auf mehreren Ebenen einsetzen ;'D

Antwort
von TeeTier, 31

Kurze Antwort: Nein, das reicht noch lange nicht. Genau diese Art von Lücken sind typisch, um mehr oder weniger Vollzugriff auf Webserver zu bekommen.

Lange Antwort: Was machst du, wenn ich einen Request mit "?site=admin" absetze? Dann wird "./include/admin.php" eingebunden. (Es sei denn, kritische Dateien liegen außerhalb vom "include" Verzeichnis, und alle darin enthaltenen Dateien sind sowieso unverfänglich.)

Ich suche solche Fehler u. a. beruflich und habe gerade erst gestern eine Website für einen Kunden untersucht, bei der es um die Absicherung eines Gewinnspiels ging, und dort fand ich exakt den gleichen Fehler, wie in deinem Snippet. :)

Da beim Kunden die Passwortüberprüfung vor dem einbinden der "admin.inc.php" stattfand, konnte ich ungehindert auf die Adminoberfläche zugreifen, einfach indem ich nach dem Absenden des Gewinnspielformulars in der Url "/gewinnspiel/vielen_dank" durch "/gewinnspiel/admin" ersetzte.

(Das wurde dann zwar mit mod_rewrite umgeschrieben, aber ist im Grunde genommen mit deinem Code identisch.)

Ich poste diesen Link hier gefühlt jede Woche, aber ich denke, das ist sowohl für dich, als auch für alle anderen Mitleser sehr interessant:

https://www.owasp.org/index.php/Cheat\_Sheets

Dort werden die wichtigsten und gängigsten Sicherheitslücken besprochen, die für weit über 90% aller Servereinbrüche und Hacks verantwortlich sind. Lies dir bitte in Ruhe alle Themen durch, die für dich interessant klingen, und verfolge nach Möglichkeit auch die verlinkten Artikel und Papers.

Viel Spaß damit! :)

PS: Sehr schön, dass du dir Gedanken um die Absicherung deines Servers machst! Sollte eigentlich selbstverständlich sein, aber wird leider zu oft unterschätzt. ><

Kommentar von TeeTier ,

PS: Da man bei solchen Sachen als Angreifer oft nur raten kann, wie die einzubindende Datei heißt, nutze ich dafür ein Skript, was die gängigsten paar 100 Namen für Administrator-Oberflächen durchprobiert. (admin, administrator, administrate, moderate, control, ... das ganze in verschiedenen Sprachen und automatisiert Groß-, Klein- und Mischschreibweise)

Der Einsatz solcher Skripte ist sehr praktisch, dauert nur Sekunden um tausende von Anfragen zu senden und bei meiner Arbeit auf Kundenwunsch natürlich völlig legal, aber fremde Server sollte man nicht ungefragt damit "testen", sonst wird das Ganze evtl. als Angriff gewertet ... mit allen rechtlichen Konsequenzen!

Wie auch immer ... Kriminelle scheren sich nicht um Gesetze, und solche Testskripte - wenn man sie sich nicht gerade selber schreibt - gibt es zu Hauf kostenlos im Netz, wobei die natürlich ebenfalls absolut legal sind, und von Pen-Testern und anderen Sicherheitsforschern oder einfach nur Admins benutzt werden. Aber wie gesagt: Kriminelle scheren sich nicht um Gesetze. Also gib dir Mühe beim Absichern deiner Website! :)

Kommentar von Dereta ,

Entschuldigung vorweg aber irgendwie hat das mit dem automatischen ausrichten nicht so ganz geklappt..

Also meine Admin seite kann zwar darüber geladen werden, jedoch wird bevor irgendwas angezeigt wird immer das UserLevel (benutzer Rang (Gast, Benutzer, Techniker, Admin, ...)) abgefragt. Dieser wird bei jeder Aktualisierung der Seite neu aus der Datenbank geladen und gesetzt.

Wo liegen da die gravierensten Fehler oder allgemeine fehler?

Folgendermaßen sieht das ganze aus:

// Refresh User
if (isset($_SESSION["USER"]))
{
$qUser = $SQL_TASK->query("SELECT * FROM " . $SQL_CONFIG["DATABASE"] . ".user WHERE id = '" . $_SESSION["USERID"] . "' LIMIT 1");
if ($qUser->num_rows)
{
$qUserData = $qUser->fetch_assoc();
if ($qUserData["status"] != "BAN")
{
$_SESSION["USERID"] = $qUserData["id"];
$_SESSION["USER"] = $qUserData["user"];
$_SESSION["PASS"] = $qUserData["password"];
$_SESSION["MAIL"] = $qUserData["userMail"];
$_SESSION["RANK"] = $qUserData["userGroup"];
// Überprüft wird es auf wichtigen seiten dann so:

if (getUserLevel() >= 10)
{
echo($NOTIFICATION["INFO"] . "Inhalt für Admin ETC");
}
else
{
echo($NOTIFICATION["INFO"] . "<br>Keine Berechtigung!");
}
// Und hier noch die Funktion
function getUserLevel($userRank)
{
$userRank = (empty($userRank) == false ? $userRank : $_SESSION["RANK"]);
include("./config/config.php");
if (userRankExist($userRank))
return $USER_RANKS[$userRank][0];
else
return "0";
}
Kommentar von Dereta ,

// Konnte es leider nicht mehr bearbeiten. Deswegen hier nochmal übersichtlicher.

Meine Admin seite kann zwar darüber geladen werden, jedoch wird
bevor irgendwas angezeigt wird immer das UserLevel (benutzer Rang (Gast,
Benutzer, Techniker, Admin, ...)) abgefragt. Dieser wird bei jeder
Aktualisierung der Seite neu aus der Datenbank geladen und gesetzt.

Wo liegen da die gravierensten Fehler oder allgemeine fehler?

Folgendermaßen sieht das ganze aus:

// Refresh User
if (isset($_SESSION["USER"]))
{
$qUser = $SQL_TASK->query("SELECT * FROM " . $SQL_CONFIG["DATABASE"] . ".user WHERE id = '" . $_SESSION["USERID"] . "' LIMIT 1");
if ($qUser->num_rows)
{
$qUserData = $qUser->fetch_assoc();
if ($qUserData["status"] != "BAN")
{
$_SESSION["USERID"] = $qUserData["id"];
$_SESSION["USER"] = $qUserData["user"];
$_SESSION["PASS"] = $qUserData["password"];
$_SESSION["MAIL"] = $qUserData["userMail"];
$_SESSION["RANK"] = $qUserData["userGroup"];
}
}
}
// Refresh User END
// Überprüft wird es auf wichtigen Seiten (u.a. auch Admin Seite) dann so:

if (getUserLevel() >= 10)
{
echo($NOTIFICATION["INFO"] . "Inhalt für Admin ETC");
}
else
{
echo($NOTIFICATION["INFO"] . "<br>Keine Berechtigung!");
}
// Und hier noch die Funktion
function getUserLevel($userRank)
{
$userRank = (empty($userRank) == false ? $userRank : $_SESSION["RANK"]);
//Include der Config, da dieser hier irgendwie nicht geladen ist
include("./config/config.php");
if (userRankExist($userRank))
return $USER_RANKS[$userRank][0];
else
return "0";
}
Antwort
von Ceins, 34

ich persönlich mag es nicht superglobale variablen direkt im code zu benutzen, wenn es vermeidbar ist.

wie wäre es mit :

if(isset($_GET['site']) && $_GET['site'] != ''){
$site = $_GET['site'];
}

und  $_GET['site'] im code austauschen mit $site.

Sonst habe ich keine "kritischen" Dinge bemerkt.

Kommentar von fluffiknuffi ,

Ist übrigens das gleiche wie:

if(! empty($_GET['site'])){
$site = $_GET['site'];
}
Kommentar von Ceins ,

Stimmt, allerdings finde ich meine Methode sprechender und einleuchtender :)

Kommentar von fluffiknuffi ,

Jeder wie er mag :)

Keine passende Antwort gefunden?

Fragen Sie die Community

Weitere Fragen mit Antworten