How secure is file reading and writing?

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
User avatar
WingedSpectre
Forum Newbie
Posts: 6
Joined: Thu May 21, 2009 8:27 pm
Location: Milton Keynes, UK

How secure is file reading and writing?

Post by WingedSpectre »

Hi,
I am trying to create a mechanism whereby known users are granted access to particular files. I was wondering if anyone knows of any inherent security issues in the method I've chosen to grant access, which is to directly modify the Apache Server .htaccess file from within my PHP script:

Code: Select all

 
$accessMod = "\r\nAllow from " . $_SERVER["REMOTE_ADDR"];
$accessFile = fopen("downloads/.htaccess","a");
if($accessFile) {
    flock($accessFile, LOCK_EX);
    fwrite($accessFile,$accessMod);
    flock($accessFile,LOCK_UN);
    fclose($accessFile);
}
My initial thought is this is safe because no-one can access the underlying code and even if they could then it would be just as easy for them to put in their own fopen/fwrite statements as it would to hijack mine, but I don't know much about PHP and feel a bit shaky about opening an important file through scripting, so thought I'd ask just in case :mrgreen:
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: How secure is file reading and writing?

Post by kaisellgren »

Is there a reason why you cannot let PHP handle the access to your files? You could store the access details on a database separately for each file and serve the files through PHP.
User avatar
WingedSpectre
Forum Newbie
Posts: 6
Joined: Thu May 21, 2009 8:27 pm
Location: Milton Keynes, UK

Re: How secure is file reading and writing?

Post by WingedSpectre »

Hi Kai, thanks for the reply :mrgreen:
There isn't any reason I couldn't let PHP handle access rights except for my ignorance. If a user enters a filename directly into their address bar, could PHP block this access without relying on an .htaccess file?
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: How secure is file reading and writing?

Post by kaisellgren »

WingedSpectre wrote:Hi Kai, thanks for the reply :mrgreen:
There isn't any reason I couldn't let PHP handle access rights except for my ignorance. If a user enters a filename directly into their address bar, could PHP block this access without relying on an .htaccess file?
No, but you can put these files outside of the document root. For example:

Code: Select all

/home/account/files/file.zip
/home/account/public_html/load_file.php?file=file.zip
Then the load_file.php obviously checks for the correct credentials. You may want to show us your implementation of this once it is finished, so, we can have a look at it, too.
User avatar
WingedSpectre
Forum Newbie
Posts: 6
Joined: Thu May 21, 2009 8:27 pm
Location: Milton Keynes, UK

Re: How secure is file reading and writing?

Post by WingedSpectre »

Ah OK! I should have spotted that but because I've been learning-as-I-go I didn't know how to serve files from outside the doc root at the time when I was thinking of storing them there. But I do now so should be able to do this with a bit of fiddling. I will post my code once it's done. Thanks :mrgreen:
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: How secure is file reading and writing?

Post by kaisellgren »

Serving files is nothing spectacular. Basically you need to output the correct headers (an example of serving a PNG file):

Code: Select all

$data = file_get_contents('/home/account/files/file.png');
header('Content-Type: image/png');
header('Content-Length: '.strlen($data));
header('X-Content-Type-Options: nosniff');
echo $data;
 
That is a simple demonstration. I did not run the code, so, it may not be error free. You could use one header call, implement download feature, and echo iteratively chunk by chunk for large files.
User avatar
WingedSpectre
Forum Newbie
Posts: 6
Joined: Thu May 21, 2009 8:27 pm
Location: Milton Keynes, UK

Re: How secure is file reading and writing?

Post by WingedSpectre »

I am done. Took me a while because I wanted to add a Challenge/Response login (thanks to Maugrim_The_Reaper's tutorial) but here is the main code from my authentication script:

Code: Select all

<?php
session_start();
//connect to database
$conn = mysql_connect('localhost', 'root', '') or die('Could not connect to database');
mysql_select_db('mytestdb', $conn) or die ('Can\'t use mytestdb : ' . mysql_error());
 
//minor filtration
if(isset($_POST['response']) && !empty($_POST['response']) && !ctype_alnum($_POST['response']))
{
    die('Bad Input: Response not alphanumeric!');
}
if(isset($_POST['password']) && !empty($_POST['password']) && !ctype_alnum($_POST['password']))
{
    die('Bad Input: Password not alphanumeric!');
}
 
//timestamp validation
$result = mysql_query("select challenge from challenge_record where sess_id = '" . session_id() . "' and timestamp > " . time()) or die("Invalid query: " . mysql_error());
if(mysql_num_rows($result) == 0)
{
    header('Location: timedout.php');
}
$c_array = mysql_fetch_assoc($result);
 
//run through passwords to find one that produces an expected/actual response match
$result = mysql_query("select password, filename from user_data") or die("Invalid query: " . mysql_error());
if(mysql_num_rows($result) == 0)
{
    header('Location: nodata.php'); //this should never be reached
}
require_once('sha256.inc.php'); //Using feyd's SHA256 PHP implementation
$bool = false;
while($row = mysql_fetch_assoc($result)) {
    $response_string = $row['password'].':'.$c_array['challenge'];
    $expected_response = SHA256::hash($response_string);
    if($_POST['response'] == $expected_response) {
        $bool = true;
        break;
    }
}
 
//if not already authenticated do an extra run through for non-JavaScript users
if(!$bool && isset($_POST['userpass']) && !empty($_POST['userpass'])) {
    $result = mysql_query("select password, filename from user_data") or die("Invalid query: " . mysql_error());
    if(mysql_num_rows($result) == 0)
    {
        header('Location: usernotexist.php'); //unreachable in theory
    }
    while($row = mysql_fetch_assoc($result)) {
        if(SHA256::hash($_POST['userpass']) == $row['password']) {
            $bool = true;
            break;
        }
    }
}
 
if($bool) {
    //if response matches then serve the appropriate file
    header("Cache-Control: no-cache");
    header("Pragma: no-cache");
    header("Content-Type:application/zip");
    header("Content-Disposition:attachment; filename=secret.zip");
    header("Content-Description: File Transfer");
    readfile("../dir/outside/root/".$row['filename']);
} else {
    //otherwise fail
    header('Location: badlogin.php');
}
?>
I'd like it to be more efficient but can't see a way to miss out unnecessary hash calculations without the addition of usernames, which would be undesirable. I don't think the inefficiency is overly criminal though :dubious:

Otherwise, on the security front, I think everything is OK :mrgreen:
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: How secure is file reading and writing?

Post by kaisellgren »

Put an exit(); after the line 21. The same applies to line 47. Just in case.

Put the x content type options nosniff to your header calls' list. This makes IE to not sniff the content (which means it respects the content type).

Btw, using empty() and isset() on the same statement is pretty much pointless as isset() is included in empty(). When you call empty(), it first checks with isset() and then it checks that != ''.

When someone adds an entry in your database, this entry may contain malicious data and your line 64 uses this column data directly. At least strip null bytes and use basename(), but I recommend running a regular expression replace for stripping out non-wanted characters.
User avatar
WingedSpectre
Forum Newbie
Posts: 6
Joined: Thu May 21, 2009 8:27 pm
Location: Milton Keynes, UK

Re: How secure is file reading and writing?

Post by WingedSpectre »

kaisellgren wrote:Put an exit(); after the line 21. The same applies to line 47. Just in case.
OK, I don't understand the reason but will do. Is this in case the server fails to follow the re-direct and keeps executing the script or is there some way a malicious user can do malicious things? (Also I suppose this means I should add an exit() after line 29 and possibly even 67).
kaisellgren wrote:Put the x content type options nosniff to your header calls' list. This makes IE to not sniff the content (which means it respects the content type).
I thought it wouldn't matter with ZIPs but I guess, either way, it's better to be safe than sorry :mrgreen:
kaisellgren wrote:Btw, using empty() and isset() on the same statement is pretty much pointless as isset() is included in empty(). When you call empty(), it first checks with isset() and then it checks that != ''.
Thanks for that tip. This is straight out of the tutorial and I hadn't thought to check it.
kaisellgren wrote:When someone adds an entry in your database, this entry may contain malicious data and your line 64 uses this column data directly. At least strip null bytes and use basename(), but I recommend running a regular expression replace for stripping out non-wanted characters.
Oops yeah I'll change that. In theory it will never happen because passwords and file locations will be updated by an administrator, but I will do it in case of unforeseen circumstances!

Thank you very much for the input.

One last(!) thing, I noticed on another thread discussing Login using Sessions you posted as follows:
kaisellgren wrote:
theorok wrote:

Code: Select all

$key = md5(uniqid(rand(), TRUE));
$key .= 'AVS1976JER1';
$token = md5($key);
The token is not very random.
Did you mean it's not very random because MD5 is only 128-bit or because of the usage of uniqid() or rand()?
Because in my code the Challenge part of the Challenge/Response is generated like this:

Code: Select all

$challenge = SHA256::hash(uniqid(mt_rand(), true));
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: How secure is file reading and writing?

Post by kaisellgren »

WingedSpectre wrote:Is this in case the server fails to follow the re-direct and keeps executing the script or is there some way a malicious user can do malicious things?
The header() -function sends data to the output buffer just like echo() does. Neither of them stops the execution, which seems to be what you want, right? It is easy to forget that your script is still executing and you may output something private in the end of the file without remembering this.
WingedSpectre wrote:I thought it wouldn't matter with ZIPs but I guess, either way, it's better to be safe than sorry :mrgreen:
It does not matter what content you have there. It is an easy place to execute some XSS, for instance, although it only works for IE.
WingedSpectre wrote:In theory it will never happen because passwords and file locations will be updated by an administrator,
If an attacker has an access to your database, but not to your filesystem - he can modify the filename and therefore, this is one simple place to get an access to your filesystem, too. Besides, you never know if you alter your application and you allow other people upload files later. This might happen two years later, and at that point you no longer remember that your "protection" relied on the fact that no one else could alter your data.
WingedSpectre wrote:Did you mean it's not very random because MD5 is only 128-bit or because of the usage of uniqid() or rand()?
No, MD5 could be used, although not the best option. The function uniqid() essentially uses the same RNG (random number generator) than what rand() uses. They both generate their random output using an LCG (linear congruential generator), which generates poor random output. The same thing applies to mt_rand(). On Unix -like platforms, it is simple to fetch decent random data by reading the /dev/urandom file with fopen().
WingedSpectre wrote:Because in my code the Challenge part of the Challenge/Response is generated like this:

Code: Select all

$challenge = SHA256::hash(uniqid(mt_rand(), true));
I see that famous sentence everywhere. First you generate random data with the Mersenne Twister, which is a twisted GFSR (generalised feedback shift register) that produces poor random. Then you use uniqid() which generates a "unique" identifier based on a few facts such as the time, the time on microseconds, the current state of the internal LCG and the previously generated poor random data. After all this, you hash it with SHA-256. It is hard to say for sure, but I have estimated that about 10% of that data is crud. However, that line of code produces poor random results. The hash function is okay, but the entire contents inside of it should be replaced by some data gathered from the /dev/urandom (or CSP on Windows). Then your challenge generation approach meets all requirements sufficiently. I suggest you to use a strength of 256-bits as using more is useless due to the underlying 256-bit hash function. 256-bits is equal to 32 bytes. So use fopen() to read /dev/urandom and read 32 bytes and close the file. Now you have good random data to pass into your SHA-256.
User avatar
WingedSpectre
Forum Newbie
Posts: 6
Joined: Thu May 21, 2009 8:27 pm
Location: Milton Keynes, UK

Re: How secure is file reading and writing?

Post by WingedSpectre »

kaisellgren wrote:The header() -function sends data to the output buffer just like echo() does. Neither of them stops the execution,
Ah OK, I'm not very sure on the flow of control in this scenario. I thought putting in a 'Location' header would stop the remainder of the current script from ever being reached because that's what appears to happen in a browser i.e. if I follow header() with an echo() I will not see the latter's output. But this echo() and subsequent script output is actually put into a buffer where it takes up resources briefly behind-the-scenes on the server (I guess?) or possibly it goes through in the HTTP response and takes up some user resources as well... either way, yes, exit() is what I want to do!
kaisellgren wrote:
WingedSpectre wrote:

Code: Select all

$challenge = SHA256::hash(uniqid(mt_rand(), true));
I see that famous sentence everywhere.
Haha, after I posted that I found your blog entry All About Randomness And Entropy and thought "doh! I asked that too soon". I shall test out your phptalk_rand() function on my host's server to see what is available. Thanks again Kai!
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: How secure is file reading and writing?

Post by kaisellgren »

WingedSpectre wrote:if I follow header() with an echo() I will not see the latter's output. But this echo() and subsequent script output is actually put into a buffer where it takes up resources briefly behind-the-scenes on the server (I guess?)
No matter what you type in header() or how many times you call it, the rest of the PHP code is executed normally. Whether you see something or not, that depends on a few aspects and often the rest of the output after header() can be seen by looking at the HTTP response headers if not by looking at the website.
gcreddy
Forum Newbie
Posts: 10
Joined: Tue Sep 08, 2009 8:26 am

Re: How secure is file reading and writing?

Post by gcreddy »

Hey kaisellgren!

That was a very good link!! It helped me a lot. I now have to figure a way to make it work on my code. I was looking for something like this. Thank you very very much! :)
Post Reply