Critical security issue

[sci2tech]

Backup engine can be used to upload a symlink to an arbitrary file. Of course, that file must be accessible and readable for vuxxxx user resulting a minor issue.
Ex:
download last available backup
unpack
create in htdocs a simlink to /etc/passwd
upload in backup folder
call http://site.tld/symlink_name and you will get passwd.

PS. My trac account is unusable that’s why I use forum.

[gOOvER]

(08-29-2010 08:35 PM)sci2tech Wrote:  PS. My trac account is unusable that’s why I use forum.

Did you activate your account?? Maybe thats the problem. Elso write malte a PM, that he can fix your account.

I will open a Ticket for you

---

Open a Ticket: http://isp-control.net/ispcp/ticket/2440

[sci2tech]

Please upgrade as critical and advise everybody to disable backup on all costumers. It is not a minor issue, it`s a BIGGG security whole because I was able to obtain total control to server. I`ll not provide yet a how to for obvious reason.
To disable backup you need to modify in ispcp-dmn-mngr line:

Code:

sub dmn_restore_data {

in

Code:

sub dmn_restore_data {
return 0;

because just setting backup to no wont prevent engine to restore a crafted backup

[sci2tech]

Indeed you need to make read only the backup folder but also change ownership of backup folder and to contained archives, and make them too read only.
Sorry to respond here not in trac but I can`t do that.

[kilburn]

There are 2 ways to handle this issue:

1. Deal with the untarring and (fixed) permission handling in an inaccessible directory, mv the properly checked result to the user folder
2. Implement backup signing (we can easily implement a signature file which is calculated by encrypting the md5 hash of the archive file).

I'm strong in favour of the 2nd option, but open to your opinions...

[sci2tech]

A quick fix, that leave backup functional and prevent issue (diff against current trunk):

Code:

Index: ispcp-backup-all
===================================================================
--- ispcp-backup-all    (revision 3238)
+++ ispcp-backup-all    (working copy)
@@ -190,10 +190,10 @@
            return $rs;
        }

-        $rs = setfmode("$db_backup_file", $domain_uid, $domain_gid, 0660);
+        $rs = setfmode("$db_backup_file", 0, 0, 0444);
        if( $rs != 0 ){
-            push_el(\@main::el, 'backup_sql()', "Domain $dmn_name: ERROR: Can not chmod 0660 uid: $domain_uid gid: $domain_gid file $db_backup_file!");
-            send_error_mail('backup_sql()', "Domain $dmn_name: ERROR: Can not chmod 0660 uid: $domain_uid gid: $domain_gid file $db_backup_file!");
+            push_el(\@main::el, 'backup_sql()', "Domain $dmn_name: ERROR: Can not chmod 0444 uid: 0 gid: 0 file $db_backup_file!");
+            send_error_mail('backup_sql()', "Domain $dmn_name: ERROR: Can not chmod 0444 uid: 0 gid: 0 file $db_backup_file!");
            unlink($db_backup_file);
            return $rs;
        }
@@ -273,10 +273,17 @@
            #

            if (! -d $dmn_backup_dir) {
-                $rs = make_dir($dmn_backup_dir, $domain_uid, $domain_gid, 0770);
+                $rs = make_dir($dmn_backup_dir, 0 ,0, 0555);
                return $rs if ($rs != 0);
            }

+            #todo test mode and adjust only if necessary
+            $rs = setfmode($dmn_backup_dir, 0, 0, 0555);
+            if ($rs != 0){
+                push_el(\@main::el, 'backup_all_engine()', "Domain $dmn_name: Error while changing mode to 0555 uid: 0: 0 for $dmn_backup_dir!");
+                send_error_mail('backup_all_engine()', "Domain $dmn_name: Error while changing mode to 0555 uid: 0 gid: 0 for $dmn_backup_dir!");
+            }
+
            if ($zip =~ '^(bzip2|gzip|lzma|xz)$') {
                my $extension = undef;

@@ -321,10 +328,10 @@
                        send_error_mail('backup_all_engine()', "Domain $dmn_name: Error while executing $cmd_mv -f $www_dir/$backup_filename $dmn_backup_dir!");
                    }

-                    $rs = setfmode("$dmn_backup_dir/$backup_filename", $domain_uid, $domain_gid, 0660);
+                    $rs = setfmode("$dmn_backup_dir/$backup_filename", 0, 0, 0444);
                    if ($rs != 0){
-                        push_el(\@main::el, 'backup_all_engine()', "Domain $dmn_name: Error while changing mode to 0660 uid: $domain_uid gid: $domain_gid for $dmn_backup_dir/$backup_filename!");
-                        send_error_mail('backup_all_engine()', "Domain $dmn_name: Error while changing mode to 0660 uid: $domain_uid gid: $domain_gid for $dmn_backup_dir/$backup_filename!");
+                        push_el(\@main::el, 'backup_all_engine()', "Domain $dmn_name: Error while changing mode to 0444 uid: 0 gid: 0 for $dmn_backup_dir/$backup_filename!");
+                        send_error_mail('backup_all_engine()', "Domain $dmn_name: Error while changing mode to 0444 uid: 0 gid: 0 for $dmn_backup_dir/$backup_filename!");
                    }

                } else { # some error occurred

[sci2tech]

In addition you need to also patch ispcp_dmn_mngr:

Code:

Index: ispcp-dmn-mngr
===================================================================
--- ispcp-dmn-mngr    (revision 3238)
+++ ispcp-dmn-mngr    (working copy)
@@ -1605,7 +1605,7 @@
    $rs = make_dir("$www_dir/$dmn_name/phptmp", $sys_user, $httpd_gid, 0770);
    return $rs if ($rs != 0);

-    $rs = make_dir("$www_dir/$dmn_name/backups", $sys_user, $httpd_gid, 0770);
+    $rs = make_dir("$www_dir/$dmn_name/backups", 0, 0, 0555);
    return $rs if ($rs != 0);

    $rs = make_dir("$www_dir/$dmn_name/errors", $sys_user, $sys_group, 0775);
@@ -1914,7 +1914,7 @@
    }

    if(! -d "$www_dir/$dmn_name/backups") {
-        $rs = make_dir("$www_dir/$dmn_name/backups", $sys_user, $httpd_gid, 0770);
+        $rs = make_dir("$www_dir/$dmn_name/backups", 0, 0, 0555);
        return $rs if ($rs != 0);
    }

[iwik]

I don't think that making users backups to be readable by each other (everyone) is good idea.

(08-30-2010 01:35 AM)sci2tech Wrote:  A quick fix, that leave backup functional and prevent issue (diff against current trunk):

Code:

-                    $rs = setfmode("$dmn_backup_dir/$backup_filename", $domain_uid, $domain_gid, 0660);
+                    $rs = setfmode("$dmn_backup_dir/$backup_filename", 0, 0, 0444);

[kilburn]

Quote:I don't think that making users backups to be readable by each other (everyone) is good idea.

There's no problem with that because user's folders are only accessible by them and the webserver (apache) user. Also notice that we use suexec so both cgi's/php's are executed as the website user, not the apache user.

[joximu]

I move this to the other section (security issues)

---

hm, maybe better in the internal area?