Download and general security question

[eluser]BobbyB[/eluser]
Hi boys and girls,
I have a question on how to secure user downloads and keep unauthorized users from downloading other users' stuff.

Right now I have follwowing code which is supposed to stop users that are logged in to guess other users' directories and download files from there.

This is my download function:

Code:

function download(){

check_login(); //////Here I check if a user is logged in at all(Helper). If not he will be redirected.

/////Now I get the users root directory and the file name/id.
$download = $this->input->get_post('download', TRUE);
$root = $this->input->get_post('root', TRUE);

///Now I check if the root directory is the same as the one saved in the session if not the user will be redirected
if ($root!=$this->session->userdata('root')){redirect('');}

.........

Is this a good way to do this?
Any comments welcome.
I am quite new to php/codeigniter so please dont hit me with a stick

Thanks in advance!

[eluser]TheFuzzy0ne[/eluser]
I'm sure I'm probably missing something here, but what exactly is 'root' used for? I don't understand it's role.

[quote author="BobbyB" date="1239311624"]I am quite new to php/codeigniter so please dont hit me with a stick [/quote]

You're OK there, because I'm fresh out of sticks. I just tend to lurk in the shadows and pounce with my ninja-like reflexes, and beat people half-to-death with the [url="http://ellislab.com/codeigniter/user-guide/"]user guide[/url].

[eluser]BobbyB[/eluser]
Hi Fuzzy,
thanks for taking some time.

"root" stands for the directory which is created for each user once registered.
It is sth. like their "home directory".

Users will then be able to log in, upload files and also their invoices will be stored in a sub-directory of "root".

Of course I dont want userA to download files that are supposed for userB only.
So I thought I would just check the "root" which is sent via post against the root that I stored in the session.

I also thought about just getting the "root" from the session only.
I was just wondering if I am missing some fundamental stuff.

Cheers&Thanks;

[eluser]TheFuzzy0ne[/eluser]
I don't understand the need to send the root via post at all. Is there any reason why it can't just be used from the userdata as is? I don't understand the inner-workings of your application, but I would have thought you'd be able to do that.

[eluser]BobbyB[/eluser]
Hi,
as I said, I am also considering getting the root directly from the session.
Would this be the best way to do this?

What do you mean by "..used from the userdata.."?
Thanks for your patience.

Greetings

[eluser]TheFuzzy0ne[/eluser]
The line:

Code:

if ($root!=$this->session->userdata('root')){redirect('');}

You're comparing the $_POST['root'] to the value stored in the session (userdata). I don't understand why this comparison is necessary, or why you even need to submit the value via a form since the data is already stored within the session.

There's probably something I'm missing here, which is why it doesn't make sense to me. It seems a bit like knocking on someone's door to ask them what their house number is, when the house number is already on the door you've just knocked on, if you catch my drift.

[eluser]BobbyB[/eluser]
Yeah, I got your point.

So I will use the "root" value directly from the session then.

uhhhuahuh... I sometimes just get confused by all the different ways and possibilities that are available.
Then I have to ask somebody, who has more experience than me for some advice

Thank you very much for clearing this one up for me.

Keep it up!

Bob

[eluser]TheFuzzy0ne[/eluser]
Please don't take what I say as arrogance. I'm just not sure I fully understand the problem, (that happens to me a lot).

If I'm missing an important factor here, please tell me. I don't understand why you submit a form for the file download, rather than work with the URI, but again, that doesn't mean your not doing it the right way. I'm just saying how I would probably do it, which is the way that makes sense to me, it doesn't mean it's the best way.

Just out of interest, is there a specific reason you decided to submit the request via a form? I'm just curious.

[eluser]BobbyB[/eluser]
Ok,
after having gotten some sleep, I will now try to explain some more and hope to be clearer about this whole thing.

My website will run on the great MochaUI application built by Greg Houston which is based on mootools.
A user can register himself and will(after confirming his data) then be able to upload his files(sound files) via swfupload.
So each user will get his own account where he can log in and will be presented his downloads/invoices/personal data.
All of this is done via ajax because I dont want any page reloads.

When a user registers himself, a directory(and sub-directory for his invoices) based on his username is automatically created on my server.
This directory is what I earlier referred to as "root".

So each user has his own directory where his/her(dont want to be sexist here) files are stored after upload.
If a user logs in he can see the files that are present in his directory.
The user information(root,username,...) is stored in the session.
It is like a basic filebrowser that reads the files from the allmighty "root" directory and also saves the filenames/size etc. to the database.

I use the "Listfiles" library for that so I will also be able to upload files via ftp and the files will still be added to the database.
It just scans the directory.
The controller looks like this:

Code:

check_login();
$root = $this->session->userdata('root');
$this->load->library('listfiles', array('wav','mp3','aiff', 'aif','aac', 'flac','ogg','rar','zip'));
$data['files'] = $this->listfiles->getFiles(realpath('filecenter/'.$root.''));



$this->load->database();
    
$sizeoffeed = sizeof($data['files']);

if ($sizeoffeed!=0){

for ($i=0;$i<$sizeoffeed;$i++)
{

$query = $this->db->query('select title from filecenter_files where file="'.$data['files'][$i]['file'].'" AND root="'.$root.'"');

if($query->num_rows()==0){
$entry = array();
    
     $entry['title'] = $data['files'][$i]['title'];
     $entry['file'] = $data['files'][$i]['file'];
     $entry['root'] = $root;
     $entry['size'] = $data['files'][$i]['size'];
     $entry['date_added'] = $data['files'][$i]['date'];
$this->db->insert('filecenter_files', $entry);
}
else{
$entry = array();

$entry['title'] = $data['files'][$i]['title'];
     $entry['file'] = $data['files'][$i]['file'];
     $entry['root'] = $root;
     $entry['size'] = $data['files'][$i]['size'];
     $entry['date_added'] = $data['files'][$i]['date'];
    
$this->db->where('file', $data['files'][$i]['file']);    
$this->db->where('root',$root);
$this->db->update('filecenter_files', $entry);


....

There is probably a lot to optimize but I havent looked over that part of my application in a while since it seems to work and there is so much more to do right now.



In that filebrowser there is also a download link for each file.

Since one can not start downloads via ajax I use a javascript function that opens a new window and passes the "root" and "filename" to the controller(the one in my first post).

Code:

...
var download_url = "http://mydomain.com/show_filecenter/download/";
        
        var id = 'download='+el.get('id');/////Gets the filename
        var root = 'root=<?=$this->session->userdata('root');?>';
        
         function targetBlank (download_url) {
                 blankWin = window.open(download_url+'?'+id+'&'+root,'_blank');
                                           }
targetBlank(download_url);
....

Thats where the code snippet in my first post is from.

So, maybe that helped you understand it a bit better? :-S

All the best!

[eluser]rogierb[/eluser]
I wouldn't pass the root since you already have it in the user session. In fact, why not store the 'id' var as flashdata so you don't have to pass any data. Then you can read both from the session.

Just my 2 cents;-)