Important security issue

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Connect as user, client or admin
2. Copy the link of a file to download
3. In the URL, change file.zip by "../index.php" or "../.htaccess"
process.php?do=download&client=client&client_id=2&url=file.zip&id=2&origin=group
&group_id=1

What is the expected output? What do you see instead?
The htacess or index is downloaded by the client.

What version of the product are you using? On what operating system?
r412 - MacOSX Safari or Chrome

Please provide any additional information below.

I will purpose a solution if I can. But don't use this system on Prod Server 
until this is secured.
Also, the $_GET variables aren't secured in multiple files, i'm very 
dissapointed because this project is very nice and useful.

Original issue reported on code.google.com by renaud.p...@gmail.com on 7 May 2013 at 12:11

[GoogleCodeExporter]

Hum, i decided to check, and i patch this very "quickly".

In process.php, i changed the function download_file()  to prevent downloading 
from an another repertory. I also decided to protect UPDATE Query and 
$_GET['client_id'] var.

I paste here my new function : 

    function download_file() {
        $this->check_level = array(9,8,7,0);
        if (isset($_GET['url']) && isset($_GET['client'])) {
            /** Do a permissions check for logged in user */
            if (isset($this->check_level) && in_session_or_cookies($this->check_level)) {
                /**
                 * If the file is being downloaded by a user, do not
                 * add +1 to the download count
                 */
                if (CURRENT_USER_LEVEL == 0) {
                    //$this->sum_sql = 'UPDATE tbl_files_relations SET download_count=download_count+1 WHERE file_id="' . $_GET['id'] .'"';
                    $this->sum_sql = sprintf('UPDATE tbl_files_relations SET download_count=download_count+1 WHERE file_id="%d"', $_GET['id']);
                    if ($_GET['origin'] == 'group') {
                        if (!empty($_GET['group_id'])) {
                            $this->group_id = $_GET['group_id'];
                            $this->sum_sql .= sprintf($this->sum_sql . " AND group_id = '%d'",$this->group_id);
                        }
                    } else {
                        $this->client_id = $_GET['client_id'];
                        $this->sum_sql .= sprintf($this->sum_sql . " AND client_id = '%d'",$this->client_id);
                    }

                    $this->sql = $this->database->query($this->sum_sql);

                    /**
                     * The owner ID is generated here to prevent false results
                     * from a modified GET url.
                     */
                    $log_action = 8;
                    $log_action_owner_id = (int)$_GET['client_id'];
                }
                else {
                    $log_action = 7;
                    $global_user = get_current_user_username();
                    $global_id = get_logged_account_id($global_user);
                    $log_action_owner_id = $global_id;
                }

                /** Record the action log */
                $new_log_action = new LogActions();
                $log_action_args = array(
                                        'action' => $log_action,
                                        'owner_id' => $log_action_owner_id,
                                        'affected_file' => (int)$_GET['id'],
                                        'affected_file_name' => mysql_real_escape_string($_GET['url']),
                                        'affected_account' => (int)$_GET['client_id'],
                                        'affected_account_name' => mysql_real_escape_string($_GET['client']),
                                        'get_user_real_name' => true,
                                        'get_file_real_name' => true
                                    );
                $new_record_action = $new_log_action->log_action_save($log_action_args);

                $urlfile = preg_replace('/^.*?([^\/\\\\]+)$/', '$1', $_GET['url']);
                $file = UPLOADED_FILES_FOLDER.$urlfile;

                if (file_exists($file)) {
                    while (ob_get_level()) ob_end_clean();
                    header('Content-Type: application/octet-stream');
                    header('Content-Disposition: attachment; filename='.basename($file));
                    header('Expires: 0');
                    header('Cache-Control: must-revalidate, post-check=0, pre-check=0');
                    header('Pragma: public');
                    header('Cache-Control: private',false);
                    header('Content-Length: ' . get_real_size(UPLOADED_FILES_FOLDER.$urlfile));
                    header('Connection: close');
                    readfile($file);
                    exit;
                }
                else {
                    header("HTTP/1.1 404 Not Found");
                    exit;
                }
            }
        }
    }

(I hope i'm not doing too much English Syntax Errors - and php as well ^^)

Original comment by renaud.p...@gmail.com on 7 May 2013 at 5:18

[GoogleCodeExporter]

is this already solved in last version?

Original comment by www.Shir...@gmail.com on 15 Jun 2013 at 11:26

[GoogleCodeExporter]

No it's not, like several other big bugs solved.

I'm waiting for a updated version to continue bug tracking...

Original comment by j.grest...@gmail.com on 15 Jun 2013 at 11:35

[GoogleCodeExporter]

When applying this fix download count stop working.
So even if a client download a file the count stay at 0.

Original comment by access_c...@hotmail.com on 4 Aug 2013 at 3:36

[GoogleCodeExporter]

This and other security fixes are now applied or on the way.
VERY close to a release, still need to figure a couple of things out.

Original comment by i...@subwaydesign.com.ar on 20 Oct 2013 at 9:22

• Changed state: Fixed