FileInclusion Protect

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
shaqa
Forum Commoner
Posts: 62
Joined: Mon Aug 13, 2007 9:11 am

FileInclusion Protect

Post by shaqa »

i dont know if i have bugs on these code below in my script.
The code below is to count downloaded file.

this code is at index.php

Code: Select all

<?php echo "<center><br><b>Downloads: </b>" . file_get_contents("data.txt") . "</center>"; ?>
file : download.php

Code: Select all

<?php
$data1 = file_get_contents("data.txt") + "1";
$fp = fopen("data.txt", "w");
fwrite($fp, $data1);
fclose($fp);
 
if(!@fopen("http://www.examplesite.com" . $_GET['file'], r)) {
echo "Broken Link!";
}else{
header("Location: http://www.examplesite.com" . $_GET['file'] . "/?id=" . $_GET['id']);
}
any solution,how do i secure?
dayyanb
Forum Commoner
Posts: 46
Joined: Wed Jan 23, 2008 12:34 am

Re: FileInclusion Protect

Post by dayyanb »

If the download is a broken link you will still increase the number of downloads.

You are vulnerable to a xss exploit because you don't check your input. Check both variables for newline or return characters, and url encode them.

Use css instead of <center>.
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: FileInclusion Protect

Post by Mordred »

dayyanb already noted the XSS issues.

If you go and actually test your code, you'll see it doesn't actually work. Not only will broken links increase the counter, but you don't distinguish between the different id-s.

Also, in the location header you should put a slash after the domain, or ensure that 'file' starts with one. Otherwise, an attacker will register http://www.example.com.au and pass the victim url like this:

Code: Select all

http://www.example.com/download.php?file=.au/evil.exe
(which will redirect to http://www.example.com.au/evil.exe )
shaqa
Forum Commoner
Posts: 62
Joined: Mon Aug 13, 2007 9:11 am

Re: FileInclusion Protect

Post by shaqa »

Mordred wrote:dayyanb already noted the XSS issues.

If you go and actually test your code, you'll see it doesn't actually work. Not only will broken links increase the counter, but you don't distinguish between the different id-s.

Also, in the location header you should put a slash after the domain, or ensure that 'file' starts with one. Otherwise, an attacker will register http://www.example.com.au and pass the victim url like this:

Code: Select all

http://www.example.com/download.php?file=.au/evil.exe
(which will redirect to http://www.example.com.au/evil.exe )
so
$data1 = file_get_contents("data.txt") + "1";
$fp = fopen("data.txt", "w") moust change to

$data1 = file_get_contents("/data.txt") + "1";
$fp = fopen("/data.txt", "w")
so the attacker can download files to my server?
dayyanb wrote:If the download is a broken link you will still increase the number of downloads.
You are vulnerable to a xss exploit because you don't check your input. Check both variables for newline or return characters, and url encode them.
Use css instead of <center>.
how do you mean to encode which line?
shaqa
Forum Commoner
Posts: 62
Joined: Mon Aug 13, 2007 9:11 am

Re: FileInclusion Protect

Post by shaqa »

Mordred wrote:dayyanb already noted the XSS issues.

If you go and actually test your code, you'll see it doesn't actually work. Not only will broken links increase the counter, but you don't distinguish between the different id-s.

Also, in the location header you should put a slash after the domain, or ensure that 'file' starts with one. Otherwise, an attacker will register http://www.example.com.au and pass the victim url like this:

Code: Select all

http://www.example.com/download.php?file=.au/evil.exe
(which will redirect to http://www.example.com.au/evil.exe )
so
$data1 = file_get_contents("data.txt") + "1";
$fp = fopen("data.txt", "w") moust change to

$data1 = file_get_contents("/data.txt") + "1";
$fp = fopen("/data.txt", "w")
so the attacker can download files to my server?
dayyanb wrote:If the download is a broken link you will still increase the number of downloads.
You are vulnerable to a xss exploit because you don't check your input. Check both variables for newline or return characters, and url encode them.
Use css instead of <center>.
any link or website for this?
dayyanb
Forum Commoner
Posts: 46
Joined: Wed Jan 23, 2008 12:34 am

Re: FileInclusion Protect

Post by dayyanb »

shaqa wrote:
Mordred wrote:dayyanb already noted the XSS issues.

If you go and actually test your code, you'll see it doesn't actually work. Not only will broken links increase the counter, but you don't distinguish between the different id-s.

Also, in the location header you should put a slash after the domain, or ensure that 'file' starts with one. Otherwise, an attacker will register http://www.example.com.au and pass the victim url like this:

Code: Select all

http://www.example.com/download.php?file=.au/evil.exe
(which will redirect to http://www.example.com.au/evil.exe )
so
$data1 = file_get_contents("data.txt") + "1";
$fp = fopen("data.txt", "w") moust change to

$data1 = file_get_contents("/data.txt") + "1";
$fp = fopen("/data.txt", "w")
so the attacker can download files to my server?
dayyanb wrote:If the download is a broken link you will still increase the number of downloads.
You are vulnerable to a xss exploit because you don't check your input. Check both variables for newline or return characters, and url encode them.
Use css instead of <center>.
any link or website for this?
url_encode
Remove \r and \n
Center text
You should also close all your html tags, use "<br />" instead of "<br>".
Post Reply