Ticket #2440 (closed security issue: fixed)

Opened 4 years ago

Last modified 4 years ago

Backup script will restore forged backup

Reported by: gOOvER Owned by: scitech
Priority: critical Milestone: ispCP ω 1.0.7
Component: Backend (Engine) Version: ispCP ω 1.0.6
Severity: Don't know Keywords:
Cc:

Description

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.

see also: http://isp-control.net/forum/thread-11531-post-86802.html

Attachments

security-2440.patch (3.4 KB) - added by benedikt 4 years ago.
patch file against ispCP Omega 1.0.6

Change History

comment:1 Changed 4 years ago by benedikt

proposed solution: read only for backup directory.

comment:2 Changed 4 years ago by benedikt

  • Milestone changed from Working to ispCP ω 1.0.7

comment:3 Changed 4 years ago by benedikt

  • Summary changed from Minor security issue to Backup script will restore forged backup

comment:4 Changed 4 years ago by benedikt

  • Priority changed from major to critical

comment:5 Changed 4 years ago by benedikt

Quick fix:

To disable backup you need to modify in ispcp-dmn-mngr line:

sub dmn_restore_data {

to

sub dmn_restore_data {
    exit 1;

comment:6 Changed 4 years ago by scitech

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

Index: ispcp-backup-all
===================================================================
--- ispcp-backup-all    (revision 3238)
+++ ispcp-backup-all    (working copy)
@@ -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: $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 0444 uid: $domain_uid gid: $domain_gid for $dmn_backup_dir/$backup_filename!");
                    }

                } else { # some error occurred

comment:7 Changed 4 years ago by scitech

Also wit sql permission fixed:

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


comment:8 Changed 4 years ago by benedikt

Patch added to trunk in r3239.

!! Caution, the patch in comment:7 does not fix the issue immediately. ispcp-backup-all needs to be run once!


The hash check proposed by Mark is a really good idea. However the hashes should be stored somewhere separated from the files where the user does not have access to, e.g. in database.

comment:9 Changed 4 years ago by scitech

In addition you need to also patch ispcp_dmn_mngr:

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);
 	}
 

comment:10 Changed 4 years ago by scitech

finally run /var/www/ispcp/engine/backup/ispcp-backup-all yes to fix folder and file permissions.

Changed 4 years ago by benedikt

patch file against ispCP Omega 1.0.6

comment:11 Changed 4 years ago by scitech

  • Owner set to scitech

comment:12 Changed 4 years ago by scitech

  • Status changed from new to assigned

comment:13 Changed 4 years ago by scitech

  • Component changed from Daemon to Backend (Engine)

comment:14 Changed 4 years ago by scitech

  • Status changed from assigned to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.