Download Counter ist er sicher ****?

[ThunderStorm1]

Mhh ich hoffe das ich eig alles abgehandel habe also von wegen RFI und LFI

if (isset($_GET['file'])&& file_exists('Downloads/'.$_GET["file"].'')  && !empty($_GET['file']))
{
	$file=$_GET['file'];
	header("Content-Type: application/octet-stream");
    header("Content-Disposition: attachment; filename=".$file."");
	readfile('http://www.julianb.de/Downloads/'.$file.'');
	
 	// Es wird überprüft ob die Datei schon in der DB vorhanden ist falls nicht wird sie hinzugefügt. 
 	$abfrage_file="SELECT filename FROM downloads WHERE filename='$file'";
 	$erg_file=mysql_query($abfrage_file);
 	$if_file_exists_inDB=mysql_num_rows($erg_file);
 
		if ($if_file_exists_inDB == 0) 
		{
		$eintragen="INSERT INTO downloads
		(filename,download_n)
		VALUES
		('{$file}','1')";
		echo  $eintragen;
		mysql_query($eintragen) or die('Beim eintragen ist ein Fehler aufgetreten. <br /> Bitte kontaktieren sie denn Admin. <a href="mailto:admin@julianb.de?subject='.$_SERVER['SCRIPT_FILENAME'].'&body='."Fehler= ".$eintragen.'">Mail an Admin</a>').mysql_error();
		}
		else 
		{
		$update_file="UPDATE downloads SET download_n  = download_n  + 1  WHERE filename='$file'";	
		mysql_query($update_file) or die('Es konnte kein Download gezählt werden. <br /> Bitte kontaktieren sie denn Admin. <a href="mailto:admin@julianb.de?subject='.$_SERVER['SCRIPT_FILENAME'].'&body='."Fehler= ".$eintragen.'">Mail an Admin</a>').mysql_error();
		}
}
else 
{
	echo "Die angeforderte Datei existirt nicht !";
}

// Copyright © ThunderStorm 2011

 

[Yaslaw]

Auf die Schnelle:
item: Alle Variablen die an die DB weitergegeben werden mit mysql_escape_string() absichern

$abfrage_file="SELECT filename FROM downloads WHERE filename='".mysql_escape_string($file)."'";

item: Alle $_GET-Variablen mit filter_input() prüfen ob das kommt was erwartet wird. Also bei $_GET['file'] ein File-Name (zB. mit FILTER_VALIDATE_REGEXP)

 

[rd4eva]

Wenn man nur vom Code ausgeht und mal etwaige Serverkonfigurationen oder sonstige Direktiven aussen vor lässt, wüsste ich nicht was mich davon abhalten sollte Beispielsweise ?file=../.htaccess oder ?file=../fooBar.php zu machen.

Mal davon abgesehen:
Da du bei readfile eine Url angegeben hast nehme ich mal an das das du allow_url_fopen aktiviert hast. Das ist Grundsätzlich nicht die beste Idee.
Es ist sowieso nicht so richtig Sinnvoll da eine Url anzugeben da du dadurch einen neuen eigentlich unnötigen Request erzeugst.

 

[saftmeister]

Kann da rd4eva nur beipflichten. Arbeite mit IDs, du hast ja ohnehin schon eine DB dafür. Und wenn du das nicht machen kannst/willst/darfst, dann wenigsten die DB-Abfrage <bevor> der Download geschickt wird. Dann wird wenigstens nur das downloadable, was in deiner DB steht und nicht jedes beliebige File, was evtl. auch durch Directory Traversal abgeholt werden kann.

 

[ThunderStorm1]

Wie meinen ****


Meinst du das so ****

$abfrage_file="SELECT filename FROM downloads WHERE filename='$file'";

readfile('http://www.julianb.de/Downloads/'.$file.'');

Aber was sollte das bringen ?

 

[saftmeister]

Den header() und readfile()-Teil am besten in den Block, in dem folgende Bedingung geprüft wird:

if ($if_file_exists_inDB == 0)

Dann wird der Download nur gestartet, wenn die Datei in deiner Datenbank eingetragen ist.