+- ispCP - Board - Support (http://www.isp-control.net/forum)
+-- Forum: ispCP Omega Development Area (/forum-1.html)
+--- Forum: Security Advisories (/forum-7.html)
+--- Thread: Critical security issue (/thread-11531.html)
Pages: 1 2
Critical security issue - sci2tech - 08-29-2010 08:35 PM
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.
RE: Minor security issue - gOOvER - 08-29-2010 08:59 PM
(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
RE: Minor security issue - sci2tech - 08-29-2010 10:58 PM
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
RE: Critical security issue - sci2tech - 08-30-2010 01:17 AM
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.
RE: Critical security issue - kilburn - 08-30-2010 01:23 AM
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...
RE: Critical security issue - sci2tech - 08-30-2010 01:35 AM
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 occurredRE: Critical security issue - sci2tech - 08-30-2010 03:55 AM
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);
}RE: Critical security issue - iwik - 08-30-2010 05:33 PM
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);
RE: Critical security issue - kilburn - 08-30-2010 06:44 PM
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.
RE: Critical security issue - joximu - 08-30-2010 07:01 PM
I move this to the other section (security issues)
hm, maybe better in the internal area?