[Scoop-dev] Exploitable security bug with file uploads.

[Hulver]

I've just checked in a CVS patch which fixes a file delete bug in the 
file upload code.

Anybody with upload_user & upload_delete permissions can delete files 
from the admin area, even if they don't have upload_admin permission.

To fix this bug either get the latest CVS version, or apply the attached 
patch.

-------------- next part --------------
Index: lib/Scoop/Admin/EditUser.pm
===================================================================
RCS file: /cvs/scoop/scoop/lib/Scoop/Admin/EditUser.pm,v
retrieving revision 1.137
diff -c -r1.137 EditUser.pm
*** lib/Scoop/Admin/EditUser.pm	14 Oct 2005 20:07:09 -0000	1.137
--- lib/Scoop/Admin/EditUser.pm	3 Oct 2006 18:38:40 -0000
***************
*** 36,50 ****
  
  	# check for delete activity
  	if ( $S->{CGI}->param('confirm_delete') && $S->{CGI}->param('delete') && $file_name ) {
- 		my $path;
- 		return 'Permission Denied' if ($uid ne $S->{UID}) || !$S->var('upload_delete');
  		if ( $S->{CGI}->param('list_type') eq 'user' ) {
! 			$path = $S->var('upload_path_user') . "$uid/";
  		} else {
! 			$path = $S->var('upload_path_admin');
  		};
  
- 		unlink "$path$file_name";
  
  		$page .= qq{<tr><td>%%norm_font%%<b><font color="red">$file_name deleted.</font></b>%%norm_font_end%%<br/>&nbsp;</td></tr>};
  	} 
--- 36,49 ----
  
  	# check for delete activity
  	if ( $S->{CGI}->param('confirm_delete') && $S->{CGI}->param('delete') && $file_name ) {
  		if ( $S->{CGI}->param('list_type') eq 'user' ) {
! 			return 'Permission Denied' if ($uid ne $S->{UID}) || !$S->var('upload_delete');
! 			$S->delete_user_file($file_name);
  		} else {
! 			return 'Permission Denied' if !($S->var('upload_admin') && $S->var('upload_delete'));
! 			$S->delete_admin_file($file_name);
  		};
  
  
  		$page .= qq{<tr><td>%%norm_font%%<b><font color="red">$file_name deleted.</font></b>%%norm_font_end%%<br/>&nbsp;</td></tr>};
  	} 
***************
*** 54,63 ****
  		my $path;
  		my $file_name_new = $S->clean_filename($S->{CGI}->param('rename_filename'));
  
- 		return 'Permission Denied' if ($uid ne $S->{UID}) || !$S->var('upload_rename');
  		if ( $S->{CGI}->param('list_type') eq 'user' ) {
  			$path = $S->var('upload_path_user') . "$uid/";
  		} else {
  			$path = $S->var('upload_path_admin');
  		};
  
--- 53,63 ----
  		my $path;
  		my $file_name_new = $S->clean_filename($S->{CGI}->param('rename_filename'));
  
  		if ( $S->{CGI}->param('list_type') eq 'user' ) {
+ 			return 'Permission Denied' if ($uid ne $S->{UID}) || !$S->var('upload_rename');
  			$path = $S->var('upload_path_user') . "$uid/";
  		} else {
+ 			return 'Permission Denied' if !($S->var('upload_admin') && $S->var('upload_rename'));
  			$path = $S->var('upload_path_admin');
  		};
  
***************
*** 111,117 ****
  	$title = 'Admin Files:' if $list_type eq 'admin';
  	my $file_list = qq{
  		<tr>
! 			<TD BGCOLOR="%%title_bgcolor%%">%%title_font%%<B>$title</B>%%title_font_end%%</TD>
  		</tr>
  		<tr>
  			<td><form method="post" name="file_$list_type" action="%%rootdir%%/user/uid:$uid/files/">
--- 111,117 ----
  	$title = 'Admin Files:' if $list_type eq 'admin';
  	my $file_list = qq{
  		<tr>
! 			<td bgcolor="%%title_bgcolor%%">%%title_font%%<b>$title</b>%%title_font_end%%</td>
  		</tr>
  		<tr>
  			<td><form method="post" name="file_$list_type" action="%%rootdir%%/user/uid:$uid/files/">
Index: lib/Scoop/Utility.pm
===================================================================
RCS file: /cvs/scoop/scoop/lib/Scoop/Utility.pm,v
retrieving revision 1.36
diff -c -r1.36 Utility.pm
*** lib/Scoop/Utility.pm	8 Feb 2006 03:42:02 -0000	1.36
--- lib/Scoop/Utility.pm	3 Oct 2006 18:40:19 -0000
***************
*** 54,59 ****
--- 55,93 ----
  	return (@files);
  }
  
+ =item * delete_user_file($file_name, $uid)
+ 
+ Deletes a specified file from $uid's files.
+ 
+ =cut
+ sub delete_user_file {
+  	my $S = shift;
+ 	my $file_name = shift;
+ 	my $uid = shift || $S->{UID};
+ 
+ 	$file_name = $S->clean_filename($file_name);
+ 
+ 	my $path = $S->var('upload_path_user') . "$uid/";
+ 
+ 	unlink "$path$file_name";
+ }
+ 
+ =item * delete_admin_file($file_name)
+ 
+ Deletes a specified admin file.
+ 
+ =cut
+ sub delete_admin_file {
+  	my $S = shift;
+ 	my $file_name = shift;
+ 
+ 	$file_name = $S->clean_filename($file_name);
+ 
+ 	my $path = $S->var('upload_path_admin');
+ 
+ 	unlink "$path$file_name";
+ }
+ 
  =item * display_upload_form()
  
  This will display an upload form, depending on the permission that the user has.