Re: [CHECKER] Probable security holes in 2.6.5

[Chris Wright <chrisw@osdl.org>]

> [BUG]
> /home/kash/linux/linux-2.6.5/drivers/scsi/aacraid/commctrl.c:419:aac_send_raw_srb: ERROR:TAINT: 413:419:Passing unbounded user value "fibsize" as arg 2 to function "copy_from_user", which uses it unsafely in model [SOURCE_MODEL=(lib,copy_from_user,user,taintscalar)] [SINK_MODEL=(lib,copy_from_user,user,trustingsink)]  [MINOR] [CAPABILTY] [PATH=] 
> 	}
> 	fib_init(srbfib);
> 
> 	srbcmd = (struct aac_srb*) fib_data(srbfib);
> 
> Start --->
> 	if(copy_from_user((void*)&fibsize,
> (void*)&user_srb->count,sizeof(u32))){
> 		printk(KERN_DEBUG"aacraid: Could not copy data size from user\n"); 
> 		rcode = -EFAULT;
> 		goto cleanup;
> 	}
> 
> Error --->
> 	if(copy_from_user(srbcmd, user_srb,fibsize)){
> 		printk(KERN_DEBUG"aacraid: Could not copy srb from user\n"); 
> 		rcode = -EFAULT;
> 		goto cleanup;
> ---------------------------------------------------------

Yup, it's protected by capable(), but...  Simple check eliminate possible
overflow.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== drivers/scsi/aacraid/commctrl.c 1.4 vs edited =====
--- 1.4/drivers/scsi/aacraid/commctrl.c	Wed Nov 19 10:38:25 2003
+++ edited/drivers/scsi/aacraid/commctrl.c	Mon Apr 19 12:02:12 2004
@@ -416,6 +416,11 @@
 		goto cleanup;
 	}
 
+	if (fibsize > FIB_DATA_SIZE_IN_BYTES) {
+		rcode = -EINVAL;
+		goto cleanup;
+	}
+
 	if(copy_from_user(srbcmd, user_srb,fibsize)){
 		printk(KERN_DEBUG"aacraid: Could not copy srb from user\n"); 
 		rcode = -EFAULT;