✔ Falsche ID-Eingaben und URL-Manipulationen abfangen

[nero_85]

Naja, man lernt nie aus! Jetzt muss ich mal schaun ob ich das irgendwo bei meine Scripts ändern muss!

 

[JohannesR]

script.php?id=1';DROP TABLE `tabelle`;--

 

[nero_85]

Ok, mittlerweile hab ichs gecheckt. Jetzt hab ich eine Frage dazu. Wenn ich das so ungefähr abschirme, ist das effektiv oder eher fürn :

function checkSaveQS($getvar){
    if(isset($getvar)){

        $chk =substr_count(strtolower($getvar),"select");
        $chk+=substr_count(strtolower($getvar),"update");
        $chk+=substr_count(strtolower($getvar),"delete");
        $chk+=substr_count(strtolower($getvar),"insert");
        $chk+=substr_count(strtolower($getvar),"drop");
        //usw. die grundsachen eben
        
        if($chk > 0){
    
            die("Es wurde eine Sicherheitslücke entdeckt! Das Script wurde beendet");
    
        }

    }
}

foreach($_GET as $get){
    if(!is_numeric($get)){
	checkSaveQS($get);
    }
}

 

[matdacat]

Ja, grundsätzlich nicht so übel. Was machst du bei großgeschriebenen SQL-Statements? ;-]

 

[Gumbo]

Verwende einfach die mysql_real_escape_string()-Funktion und alles ist gut.

 

[nero_85]

@matdacat
Das erledigt strtolower für mich

@Gumbo
Das geht natürlich auch, aber zum nachträglich ins Script zu adaptieren, wäre meins einfacher, denn da ist es dann egal, welchen $_GET-Wert ich wo benutze! Bei der von dir Vorgeschlagenen Funktion müsste ich wieder sämtliche Query's durchgehen und schaun ob ich einen $_GET-Wert benutze! Und da einige ältere Scripts von mir so um die 500 Zeilen/Seite haben ist das ziemlich Zeitaufwändig! Oder sehe ich das falsch!

PS.: Aber in Zukunft werde ich die von dir Vorgeschlagene Funktion verwenden

 

[abakuz]

hab eure Diskusion aufmerksam verfolgt und das Skript von nero_85 angepasst.
Funktioniert wunderbar...

//überprüfe ob id gesetzt, wenn nicht nimm id=10 als Standard-id
if(!isset($_GET["id"])) $id = "10";

if(isset($_GET['id']) && is_numeric($_GET['id'] )){
  $sql = mysql_query("SELECT * FROM content_title WHERE id='".$_GET['id']."'");
  $count = mysql_num_rows($sql);
    if($count == 0){
        echo "Diese Seite existiert nicht!"."</br>";
        echo '<a href="'.$_SERVER['PHP_SELF'] .'">zurück</a>';
        exit();
	}
	else{
		if(!isset($id)) $id = $_GET["id"];	
	}	
}

Was mich noch ein bisschen verunsichert ist dieser Kommentar:

@Johannes Röttger

script.php?id=1';DROP TABLE `tabelle`;--

bedeutet es, dass damit die Tabelle löschbar wäre?


Gruß abakuz

 

[JohannesR]

bedeutet es, dass damit die Tabelle löschbar wäre?

Erfasst. Wenn man Variablen nicht entsprechend escaped oder prueft, kann das passieren.

 

[Matthias Reitinger]

Was mich noch ein bisschen verunsichert ist dieser Kommentar:

script.php?id=1';DROP TABLE `tabelle`;--

bedeutet es, dass damit die Tabelle löschbar wäre?

Nein, in der Form wohl eher nicht. [phpf]mysql_query[/phpf] unterstützt die Ausführung mehrerer Anfragen in einem Aufruf nicht.

Eine meiner Meinung nach gute Lösung der SQL-Injektion-Problematik wäre die Verwendung einer Wrapperklasse für die MySQL-Funktionen, die man um eine Methode query_safe (o.ä.) erweitert:

<?php

function query_safe()
{
	// Mindestens ein Parameter benötigt
	if (func_num_args() < 1) return false;

	if (func_num_args() == 1) {
		// Nur ein Argument -> Delegation an
		// Standardfunktion
		return $this->query(func_get_arg(0));
	}

	/*
	* Verarbeitung der variablen Argumentenliste
	*/
	$args = func_get_args();

	// Erstes Arrayelement ist die Anfrage mit
	// Formatierungszeichen
	$query = array_shift($args);

	// "Entschärfung" der restlichen Argumente
	foreach ($args as $n => $arg) {
		$args[$n] = mysql_real_escape_string($arg);
	}

	// Argumente in Abfrage integrieren
	$safe_query = vsprintf($query, $args);

	// Abfrage senden
	return $this->query($safe_query);
}

?>

Ein Aufruf könnte dann beispielsweise so aussehen:

<?php

$db->query_safe('SELECT * FROM `tabelle` WHERE `foo` = \'%s\' AND `bar` = \'%d\'',
	$_REQUEST['foo'], $_REQUEST['bar']);

?>

Dieses Verfahren sollte recht robust und wenig anfällig für SQL-Injektionen sein.

(Sämtlicher Code diemal leider nicht getestet, da grad keine entsprechende Serverumgebung erreichbar ist.)

 

[the-conqueror]

Wäre es nicht wesentlich einfacher wenn du array_map verwendest?

  <?php
  function escape(&$array)
   {
    mysql_real_escape_string(htmlentities($array,ENT_QUOTES));
   }
  
  if(is_array($_POST))    { array_map('escape',$_POST);   }
  if(is_array($_GET))     { array_map('escape',$_GET);    }
  ?>

So in etwa müsste es schätze ich aussehen hab es jetzt nicht getestet.