Skip to content

Commit 7ac119c

Browse files
Bugfix: SECURITY FIX: download as zip may grant access to any files.
This bugfix removes a vulnerability bug of quixplorer from which any file in your system (which is readable to the web process) may be downloaded from your system. - Refactored downloading and access right controlling. - Fixed download and path handling. Closes #21
1 parent 6629bbb commit 7ac119c

File tree

7 files changed

+108
-99
lines changed

7 files changed

+108
-99
lines changed

Diff for: src/_include/fun_down.php

+37-70
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,7 @@
11
<?php
2-
/*------------------------------------------------------------------------------
3-
The contents of this file are subject to the Mozilla Public License
4-
Version 1.1 (the "License"); you may not use this file except in
5-
compliance with the License. You may obtain a copy of the License at
6-
http://www.mozilla.org/MPL/
72

8-
Software distributed under the License is distributed on an "AS IS"
9-
basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the
10-
License for the specific language governing rights and limitations
11-
under the License.
12-
13-
The Original Code is fun_down.php, released on 2003-01-25.
14-
15-
The Initial Developer of the Original Code is The QuiX project.
16-
17-
Alternatively, the contents of this file may be used under the terms
18-
of the GNU General Public License Version 2 or later (the "GPL"), in
19-
which case the provisions of the GPL are applicable instead of
20-
those above. If you wish to allow use of your version of this file only
21-
under the terms of the GPL and not to allow others to use
22-
your version of this file under the MPL, indicate your decision by
23-
deleting the provisions above and replace them with the notice and
24-
other provisions required by the GPL. If you do not delete
25-
the provisions above, a recipient may use your version of this file
26-
under either the MPL or the GPL."
27-
------------------------------------------------------------------------------*/
28-
/*------------------------------------------------------------------------------
29-
Author: The QuiX project
30-
quix@free.fr
31-
http://www.quix.tk
32-
http://quixplorer.sourceforge.net
33-
34-
Comment:
35-
QuiXplorer Version 2.3
36-
File-Download Functions
37-
38-
Have Fun...
39-
------------------------------------------------------------------------------*/
40-
require_once("./_include/permissions.php");
3+
require_once("_include/fun_archive.php");
4+
require_once("_include/permissions.php");
415
require_once("qxpage.php");
426

437
/**
@@ -46,52 +10,42 @@
4610
**/
4711
function download_selected($dir)
4812
{
49-
$dir = get_abs_dir($dir);
50-
global $site_name;
5113
require_once("_include/fun_archive.php");
5214
$items = qxpage_selected_items();
53-
54-
// check if user selected any items to download
55-
switch (count($items))
56-
{
57-
case 0:
58-
show_error($GLOBALS["error_msg"]["miscselitems"]);
59-
case 1:
60-
if (is_file($items[0]))
61-
{
62-
download_item( $dir, $items[0] );
63-
break;
64-
}
65-
// nobreak, downloading a directory is done
66-
// with the zip file
67-
default:
68-
zip_download( $dir, $items );
69-
}
15+
_download_items($dir, $items);
7016
}
7117

7218
// download file
7319
function download_item($dir, $item)
7420
{
75-
// Security Fix:
76-
$item=basename($item);
21+
_download_items($dir, array($item));
22+
}
23+
24+
function _download_items($dir, $items)
25+
{
26+
// check if user selected any items to download
27+
_debug("count items: '$items[0]'");
28+
if (count($items) == 0)
29+
show_error($GLOBALS["error_msg"]["miscselitems"]);
7730

78-
if (!permissions_grant($dir, $item, "read"))
79-
show_error($GLOBALS["error_msg"]["accessfunc"]);
31+
// check if user has permissions to download
32+
// this file
33+
if ( ! _is_download_allowed($dir, $items) )
34+
show_error( $GLOBALS["error_msg"]["accessitem"] );
8035

81-
if (!get_is_file($dir,$item))
36+
// if we have exactly one file and this is a real
37+
// file we directly download it
38+
if ( count($items) == 1 && get_is_file( $dir, $items[0] ) )
8239
{
83-
_debug("error download");
84-
show_error($item.": ".$GLOBALS["error_msg"]["fileexist"]);
40+
$abs_item = get_abs_item($dir, $items[0]);
41+
_download($abs_item, $items[0]);
8542
}
86-
if (!get_show_item($dir, $item))
87-
show_error($item.": ".$GLOBALS["error_msg"]["accessfile"]);
8843

89-
$abs_item = get_abs_item($dir,$item);
90-
_download($abs_item, $item);
44+
// otherwise we do the zip download
45+
zip_download( get_abs_dir($dir), $items );
9146
}
9247

93-
function _download_header($filename, $filesize = 0)
94-
{
48+
function _download_header($filename, $filesize = 0) {
9549
$browser=id_browser();
9650
header('Content-Type: '.(($browser=='IE' || $browser=='OPERA')?
9751
'application/octetstream':'application/octet-stream'));
@@ -118,4 +72,17 @@ function _download($file, $localname)
11872
exit;
11973
}
12074

75+
function _is_download_allowed( $dir, $items )
76+
{
77+
foreach ($items as $file)
78+
{
79+
if (!permissions_grant($dir, $file, "read"))
80+
return false;
81+
82+
if (!get_show_item($dir, $file))
83+
return false;
84+
}
85+
86+
return true;
87+
}
12188
?>

Diff for: src/_include/fun_extra.php

+43-21
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
<?php
22

33
require_once "_include/session.php";
4+
require_once "_include/qxpath.php";
5+
require_once "_include/str.php";
46

57
//------------------------------------------------------------------------------
68
// THESE ARE NUMEROUS HELPER FUNCTIONS FOR THE OTHER INCLUDE FILES
@@ -30,7 +32,7 @@ function get_abs_dir($path)
3032
}
3133

3234
function get_abs_item($dir, $item) { // get absolute file+path
33-
return get_abs_dir($dir).DIRECTORY_SEPARATOR.$item;
35+
return realpath(get_abs_dir($dir).DIRECTORY_SEPARATOR.$item);
3436
}
3537
/**
3638
get file relative from home
@@ -40,9 +42,15 @@ function get_rel_item($dir, $item)
4042
return $dir == "" ? $item : "$dir/$item";
4143
}
4244

43-
function get_is_file($dir, $item) { // can this file be edited?
44-
return @is_file(get_abs_item($dir,$item));
45+
/**
46+
can this file be edited?
47+
*/
48+
function get_is_file($dir, $item)
49+
{
50+
$filename = get_abs_item($dir, $item);
51+
return @is_file($filename);
4552
}
53+
4654
//------------------------------------------------------------------------------
4755
function get_is_dir($dir, $item) { // is this a directory?
4856
return @is_dir(get_abs_item($dir,$item));
@@ -182,32 +190,46 @@ function get_mime_type ($dir, $item, $query)
182190
*/
183191
function get_show_item ($directory, $file)
184192
{
193+
// no relative paths are allowed in directories
185194
if ( preg_match( "/\.\./", $directory ) )
186195
return false;
187196

188-
if ( isset($file) && preg_match( "/\.\./", $file ) )
189-
return false;
190-
191-
// dont display own directory
192-
if ( $file == "." )
193-
return false;
197+
if ( isset($file) )
198+
{
199+
// file name must not contain any path separators
200+
if ( preg_match( "/[\/\\\\]/", $file ) )
201+
return false;
202+
203+
// dont display own and parent directory
204+
if ( $file == "." || $file == ".." )
205+
return false;
206+
207+
// determine full path to the file
208+
$full_path = get_abs_item( $directory, $file );
209+
_debug("full_path: $full_path");
210+
if ( ! str_startswith( $full_path, path_f() ) )
211+
return false;
212+
}
194213

195-
if ( substr( $file, 0, 1) == "." && $GLOBALS["show_hidden"] == false )
196-
return false;
214+
// check if user is allowed to acces shidden files
215+
global $show_hidden;
216+
if ( ! $show_hidden )
217+
{
218+
if ( $file[0] == '.' )
219+
return false;
220+
221+
// no part of the path may be hidden
222+
$directory_parts = explode( "/", $directory );
223+
foreach ( $directory_parts as $directory_part )
224+
{
225+
if ( $directory_part[0] == '.' )
226+
return false;
227+
}
228+
}
197229

198230
if (matches_noaccess_pattern($file))
199231
return false;
200232

201-
if ( $GLOBALS["show_hidden"] == false )
202-
{
203-
$directory_parts = explode( "/", $directory );
204-
foreach ($directory_parts as $directory_part )
205-
{
206-
if ( substr ( $directory_part, 0, 1) == "." )
207-
return false;
208-
}
209-
}
210-
211233
return true;
212234
}
213235

Diff for: src/_include/init.php

+12-4
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,23 @@
1717
die("<B>ERROR: Your PHP version is too old</B><BR>".
1818
"You need at least PHP 4.0.0 to run QuiXplorer; preferably PHP 4.3.1 or higher.");
1919
}
20-
//------------------------------------------------------------------------------
21-
// Get Action
22-
if(isset($GLOBALS['__GET']["action"])) $GLOBALS["action"]=$GLOBALS['__GET']["action"];
23-
else $GLOBALS["action"]="list";
20+
21+
_debug("xxx3 action: " . $GLOBALS['__GET']["action"] . "/" . $GLOBALS["__GET"]["do_action"] . "/" . (isset($GLOBALS['__GET']['action']) ? "true" : "false"));
22+
if (isset($GLOBALS['__GET']["action"]))
23+
{
24+
$GLOBALS["action"]=$GLOBALS['__GET']["action"];
25+
}
26+
else
27+
{
28+
$GLOBALS["action"]="list";
29+
}
2430
if($GLOBALS["action"]=="post" && isset($GLOBALS['__POST']["do_action"])) {
2531
$GLOBALS["action"]=$GLOBALS['__POST']["do_action"];
2632
}
2733
if($GLOBALS["action"]=="") $GLOBALS["action"]="list";
2834
$GLOBALS["action"]=stripslashes($GLOBALS["action"]);
35+
_debug("xxx3 action: " . $GLOBALS['__GET']["action"] . "/" . $GLOBALS["__GET"]["do_action"] . "/" . (isset($GLOBALS['__GET']['action']) ? "true" : "false"));
36+
2937

3038
// Get Item
3139
if(isset($GLOBALS['__GET']["item"])) $GLOBALS["item"]=stripslashes($GLOBALS['__GET']["item"]);

Diff for: src/_include/permissions.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ function permissions_get ()
2929
depending the rights of the current user.
3030
3131
@param $dir Directory in which the action should happen. If this parameter is
32-
NULL the engine relys on the global permissions of the user.
32+
NULL the engine checks the global permissions of the user.
3333
3434
@param $file File on which the action should happen, if this parameter is NULL
35-
the permission engine relys on the permission of the directory.
35+
the permission engine checks the user permissions on the directory.
3636
3737
@param $action
3838
One ore more action of the action set (see permissions_get) which sould

Diff for: src/_include/qxpath.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
user, since he should only see relative pathes.
1515
1616
*/
17-
function path_f ($path)
17+
function path_f ($path = '')
1818
{
1919
global $home_dir;
2020
$abs_dir = $home_dir;

Diff for: src/_include/str.php

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
function str_startswith ( $candidate, $search_str )
4+
{
5+
return substr( $candidate, 0, strlen( $search_str) ) == $search_str;
6+
}
7+
8+
?>

Diff for: src/index.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,11 @@
7373
ob_start(); // prevent unwanted output
7474
require "./_include/fun_down.php";
7575
ob_end_clean(); // get rid of cached unwanted output
76-
download_item($current_dir, $GLOBALS["item"]);
76+
global $item;
77+
_debug("download item: $current_dir/$item");
78+
if ($item == '' )
79+
show_error($GLOBALS["error_msg"]["miscselitems"]);
80+
download_item($current_dir, $item);
7781
ob_start(false); // prevent unwanted output
7882
exit;
7983
break;

0 commit comments

Comments
 (0)