iridos / Bengelsystem

Bengelsystem is a platform to schedule shifts of volunteers primarily for jugglig conventions. Helpers register at the platform and choose shifts. The admin of the website can add new tasks and shifts.
GNU General Public License v3.0
2 stars 1 forks source link

Mehrere SQL Injections #28

Open alangecker opened 7 months ago

alangecker commented 7 months ago

an mehreren Stellen gibts die Möglichkeit von SQL injections, primär weil mysqli_real_escape_string() falsch verstanden wird.

Die Funktion ist keineswegs ein Allheilmittel zum Absichern von Inputs in nem SQL Statement, sondern escaped nur NUL (ASCII 0), \n, \r, \, ', ", and CTRL+Z. D.h. benutzt ausserhalb von nem string mit ' sichert das nich ab und es kann beliebiger SQL Code eingeschläußt werden, darf nur keine ' oder " vorkommen^^

Da SQL Injections auf die Art viel zuviele Fehlerquellen bietet, sollte mensch generell eher prepared statements nutzen.

Anfälliger Beispielcode


function HelferVonSchichtLoeschen_SchichtID($db_link, $HelferID, $SchichtID, $AdminID = 0)
{
    $HelferID = mysqli_real_escape_string($db_link, $HelferID);
    $SchichtId = mysqli_real_escape_string($db_link, $SchichtId);

    // Log vor Löschen, damit Einzelschicht abgefragt werden kann
    LogSchichtEingabe($db_link, $HelferID, $SchichtID, -1, "entfernt", $AdminID);

    // Lösche Einzelschicht
    $sql = "Delete From EinzelSchicht Where SchichtID = $SchichtID and HelferID = $HelferID limit 1;";
    //echo $sql;
    $db_erg = mysqli_query($db_link, $sql);

    return $db_erg;
}
iridos commented 6 months ago

Hi,

wir hatten das Issue eigentlich noch am gleichen Tag gelesen - sogar gemeinsam, weil wir an diesem Tag zusammen auf einem Treffen waren… nur ist dann die Beantwortung unter den Tisch gefallen.

Ein Umstellen auf PDO ist schon geplant, ich habe den entsprechenden Zweig mit dem Ticket verlinkt.

Ansonsten denke ich, ich hätte von Vorneherein lieber selbst eine simple Funktion schreiben sollen, die für Zahlen nur Zahlen zulässt und für Text nur das, was ich in Text erwarte und alles andere rückstandslos entfernt. Aber sei's drum.

Danke für den Hinweis.

Grüße, Karsten