Can a downloadable file with a .zip file extension be a problem?

Security of files with ,zip files for upload

[kiwibrit]

I'm preparing an upload page where zipped files only should be uploaded. Visitors will go to a download page with a user name and password, and expect to download zipped files. The files will not be opened on the web host server. Whilst access to the upload page will be restricted, I want to make sure I am offering reasonable security.

Can a file with a .zip extension have malicious script which can be activated in such circumstances? I am hoping not, and that I can merely check if the files have the .zip extension with something like:

$filename = 'afile.zip';
$ext = pathinfo($filename, PATHINFO_EXTENSION);

and rely on that.

Would that be sufficient? If not, what would you recommend?

[Readie]

If you simply do the "send download headers, get raw file data, print" method, you should be fine - nothing the user uploaded will be executed by the server.

The following code should do what you want to do:

<?php

$file_path = '/path/to/file/from/server/root.zip';

if(headers_sent()) {
  die('Headers Sent');
}
if(ini_get('zlib.output_compression')) {
  ini_set('zlib.output_compression', 'Off');
}
if(file_exists($file_path)) {
  header('Pragma: public');
  header('Expires: 0');
  header('Cache-Control: must-revalidate, post-check=0, pre-check=0');
  header('Cache-Control: private', false);
  header('Content-Type: application/force-download');
  header('Content-Disposition: attachment; filename="' . basename($file_path) . '";');
  header('Content-Transfer-Encoding: binary');
  header('Content-Length: ' . filesize($file_path));
  ob_clean();
  flush();
  readfile($fullPath);
} else {
  die('File Not Found');
}

?>

[kiwibrit]

I'll go over that again tomorrow morning when I am less tired to make sure I really understand it - but at first look I get the gist - and it certainly looks very neat. Many thanks.

[kiwibrit]

It seems to work well - thanks. This is the first time I have had to code the handling of files uploaded over the web, so I've taken a good look at the code. Could you explain:

if(headers_sent()) {
 die('Headers Sent');
}

For the rest, I have adapted my server-side uploadify file as shown below. I'd be grateful if you could see if what I have done looks reasonable - and also advise if the comments I have added are correct.

<?php
//This page is for handling zip files only.
if (!empty($_FILES)) {
$tempFile = $_FILES['Filedata']['tmp_name'];
$targetPath = $_SERVER['DOCUMENT_ROOT'] . $_REQUEST['folder'] . '/';
$targetFile = str_replace('//','/',$targetPath) . $_FILES['Filedata']['name'];
//not understood
if(headers_sent()) {
 die('Headers Sent');
}
//make sure zlib.output_compression is off to allow output handlers
if(ini_get('zlib.output_compression')) {
 ini_set('zlib.output_compression', 'Off');
}
// allow cache 
 header('Pragma: public');
//make permanent
 header('Expires: 0');
 header('Cache-Control: must-revalidate, post-check=0, pre-check=0');
// allow cache through proxy
 header('Cache-Control: private', false);
 header('Content-Type: application/force-download');
 header('Content-Disposition: attachment; filename="' . basename($tempFile) . '";');
 header('Content-Transfer-Encoding: binary');
 header('Content-Length: ' . filesize($tempFile));
 ob_clean();
 flush();
 $fileTypes = str_replace('*.','',$_REQUEST['fileext']);
 $fileTypes = str_replace(';','|',$fileTypes);
 $typesArray = split('\|',$fileTypes);
 $fileParts = pathinfo($_FILES['Filedata']['name']);
if (in_array($fileParts['extension'],$typesArray)) {
move_uploaded_file($tempFile,$targetFile);
echo str_replace($_SERVER['DOCUMENT_ROOT'],'',$targetFile);
 } else {
 echo 'Invalid file type.';
 }
}
?>

PS how did you get that box-effect around your code? It does make for easier reading.

[Readie]

if(headers_sent()) {
  die('Headers Sent');
}

If you've already echoed anything to the browser, using the header() function will fail and issue a warning (can't remember off the top of my head what level of warning) - this will stop the script before it reaches the header functions.

how did you get that box-effect around your code? It does make for easier reading.

[quote][pre]{code tags - break if I put them in here}[/pre][/quote]

[kiwibrit]

Got it - I like to understand code when it's passed on. I take it the rest of the code looks good? Thanks for your help.

[Readie]

Well, you seem to be assuming that several variables created from user defined input will be available, without validating their existance or data type.

The isset() function should be used to make sure that they are set, and a commonly overlooked part of validation when dealing with user defined input data is to make sure you are dealing with a string. is_string() is your friend :) prevent errors being triggered, even if display_errors is set to off in the ini file.

[rocknbil]

I'm preparing an upload page where zipped files only should be uploaded.

This is your point of danger, checking the file extension is not enough. you need to contrive a way of reading the file safely to see if it is indeed a zip file - I have no examples handy but there should be plenty out there.

Second, just a FYI, the "force download" method above is handy for cases where the browser would otherwise display the object in the browser (like a JPG or PDF, for example.) You only need to link to a zip file, as the default method of handling those files is to download. :-)