[SECURITY] CAN-2004-0109 isofs fix.

[SECURITY] CAN-2004-0109 isofs fix.

[Dave Jones]

Merged in 2.4, and various vendor kernels today..

  iDefense reported a buffer overflow flaw in the ISO9660 filesystem code.
  An attacker could create a malicious filesystem in such a way that they
  could gain root privileges if that filesystem is mounted. The Common
  Vulnerabilities and Exposures project (cve.mitre.org) has assigned the name
  CAN-2004-0109 to this issue.

Ernie Petrides came up with the following patch which I fixed up a slight
reject in to apply to 2.6. Otherwise, unchanged from the 2.4 patch.

diff against bk-HEAD from a few minutes ago.

		Dave

--- linux/fs/isofs/rock.c.orig
+++ linux/fs/isofs/rock.c
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/pagemap.h>
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
+#include <asm/page.h>
 
 #include "rock.h"
@@ -419,7 +420,7 @@ int parse_rock_ridge_inode_internal(stru
   return 0;
 }
 
-static char *get_symlink_chunk(char *rpnt, struct rock_ridge *rr)
+static char *get_symlink_chunk(char *rpnt, struct rock_ridge *rr, char *plimit)
 {
 	int slen;
 	int rootflag;
@@ -431,16 +432,25 @@ static char *get_symlink_chunk(char *rpn
 		rootflag = 0;
 		switch (slp->flags & ~1) {
 		case 0:
+			if (slp->len > plimit - rpnt)
+				return NULL;
 			memcpy(rpnt, slp->text, slp->len);
 			rpnt+=slp->len;
 			break;
+		case 2:
+			if (rpnt >= plimit)
+				return NULL;
+			*rpnt++='.';
+			break;
 		case 4:
+			if (2 > plimit - rpnt)
+				return NULL;
 			*rpnt++='.';
-			/* fallthru */
-		case 2:
 			*rpnt++='.';
 			break;
 		case 8:
+			if (rpnt >= plimit)
+				return NULL;
 			rootflag = 1;
 			*rpnt++='/';
 			break;
@@ -457,17 +467,23 @@ static char *get_symlink_chunk(char *rpn
 			 * If there is another SL record, and this component
 			 * record isn't continued, then add a slash.
 			 */
-			if ((!rootflag) && (rr->u.SL.flags & 1) && !(oldslp->flags & 1))
+			if ((!rootflag) && (rr->u.SL.flags & 1) &&
+			    !(oldslp->flags & 1)) {
+				if (rpnt >= plimit)
+					return NULL;
 				*rpnt++='/';
+			}
 			break;
 		}
 
 		/*
 		 * If this component record isn't continued, then append a '/'.
 		 */
-		if (!rootflag && !(oldslp->flags & 1))
+		if (!rootflag && !(oldslp->flags & 1)) {
+			if (rpnt >= plimit)
+				return NULL;
 			*rpnt++='/';
-
+		}
 	}
 	return rpnt;
 }
@@ -548,7 +564,10 @@ static int rock_ridge_symlink_readpage(s
 			CHECK_SP(goto out);
 			break;
 		case SIG('S', 'L'):
-			rpnt = get_symlink_chunk(rpnt, rr);
+			rpnt = get_symlink_chunk(rpnt, rr,
+						 link + (PAGE_SIZE - 1));
+			if (rpnt == NULL)
+				goto out;
 			break;
 		case SIG('C', 'E'):
 			/* This tells is if there is a continuation record */

[SECURITY] CAN-2004-0075 (was: Re: [SECURITY] CAN-2004-0109 isofs fix.)

[Marc-Christian Petersen]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

On Wednesday 14 April 2004 19:11, Dave Jones wrote:

Hi,

> Merged in 2.4, and various vendor kernels today..

Okay, now while we are at fixing security holes, is there any chance we 
can _finally_ get the attached patch in?

The Vicam USB driver in all Linux Kernels 2.6 mainline does not use the 
copy_from_user function when copying data from userspace to kernel space, 
which crosses security boundaries and allows local users to cause a denial
of service.

Already ACKed by Greg. Only complaint was inproper coding style which is done 
with attached patch ;)

ciao, Marc



[-- Attachment #2: 8009_CAN-2004-0075-usb-vicam.patch --]
[-- Type: text/x-diff, Size: 686 bytes --]

diff -urN a/drivers/usb/media/vicam.c b/drivers/usb/media/vicam.c
--- a/drivers/usb/media/vicam.c	2003-11-28 10:26:20.000000000 +0100
+++ b/drivers/usb/media/vicam.c	2004-01-15 12:10:23.000000000 +0100
@@ -653,12 +653,18 @@
 	case VIDIOCSWIN:
 		{
 
-			struct video_window *vw = (struct video_window *) arg;
-			DBG("VIDIOCSWIN %d x %d\n", vw->width, vw->height);
+			struct video_window vw;
 
-			if ( vw->width != 320 || vw->height != 240 )
+			if (copy_from_user(&vw, arg, sizeof(vw))) {
 				retval = -EFAULT;
+				break;
+			}
+
+			DBG("VIDIOCSWIN %d x %d\n", vw->width, vw->height);
 			
+			if ( vw.width != 320 || vw.height != 240 )
+				retval = -EFAULT;
+
 			break;
 		}
 

Re: [SECURITY] CAN-2004-0075 (was: Re: [SECURITY] CAN-2004-0109 isofs fix.)

[Dave Jones]

On Wed, Apr 14, 2004 at 10:30:33PM +0200, Marc-Christian Petersen wrote:

 > Okay, now while we are at fixing security holes, is there any chance we 
 > can _finally_ get the attached patch in?

Ugh, that's still lingering? I thought that was sitting in Greg's to-be-merged
tree, or had been merged already. Good job you've kept on this one.

		Dave

Re: [SECURITY] CAN-2004-0075 (was: Re: [SECURITY] CAN-2004-0109 isofs fix.)

[Greg KH]

On Wed, Apr 14, 2004 at 10:30:33PM +0200, Marc-Christian Petersen wrote:
> On Wednesday 14 April 2004 19:11, Dave Jones wrote:
> 
> Hi,
> 
> > Merged in 2.4, and various vendor kernels today..
> 
> Okay, now while we are at fixing security holes, is there any chance we 
> can _finally_ get the attached patch in?
> 
> The Vicam USB driver in all Linux Kernels 2.6 mainline does not use the 
> copy_from_user function when copying data from userspace to kernel space, 
> which crosses security boundaries and allows local users to cause a denial
> of service.
> 
> Already ACKed by Greg. Only complaint was inproper coding style which is done 
> with attached patch ;)

Eeek, I thought this one was already in the tree, very sorry about that.

I'm applying it now and will send it to Linus in a bit.

thanks for reminding me,

greg k-h

Re: [SECURITY] CAN-2004-0075 (was: Re: [SECURITY] CAN-2004-0109 isofs fix.)

[Marc-Christian Petersen]

On Wednesday 14 April 2004 22:47, Dave Jones wrote:

Hey Dave,

>  > Okay, now while we are at fixing security holes, is there any chance we
>  > can _finally_ get the attached patch in?

> Ugh, that's still lingering? I thought that was sitting in Greg's
> to-be-merged tree, or had been merged already. Good job you've kept on this
> one.

yep, still lingering. ... But Greg picked it up now and merged it :)

ciao, Marc

Re: [SECURITY] CAN-2004-0075 (was: Re: [SECURITY] CAN-2004-0109 isofs fix.)

[Marc-Christian Petersen]

On Wednesday 14 April 2004 23:27, Greg KH wrote:

Hey Greg,

> > Already ACKed by Greg. Only complaint was inproper coding style which is
> > done with attached patch ;)

> Eeek, I thought this one was already in the tree, very sorry about that.

No problem. Therefore I am here ;) ... Every now and then I take a look into 
my WOLK tree and see if there are important things not merged yet.


> I'm applying it now and will send it to Linus in a bit.
> thanks for reminding me,

np. Thank you.


ciao, Marc

[SECURITY] CAN-2004-0177 (was: Re: [SECURITY] CAN-2004-0075)

[Marc-Christian Petersen]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

On Wednesday 14 April 2004 22:30, you wrote:

Hi again,

> Okay, now while we are at fixing security holes, is there any chance we
> can get the attached patch in?

Okay, we are at it, so what's about the attached one too? ;)

In WOLK for some time too. I am not 100% sure if this is correct, but I think 
it is. Andrew? Stephen?

----------------------------------------------------------------------
CAN-2004-0177
    Solar Designer discovered an information leak in the ext3 code of
    Linux.  In a worst case an attacker could read sensitive data such
    as cryptographic keys which would otherwise never hit disk media.
    Theodore Ts'o developed a correction for this.
----------------------------------------------------------------------

ciao, Marc

[-- Attachment #2: 8009_CAN-2004-0177-ext3.patch --]
[-- Type: text/x-diff, Size: 360 bytes --]

--- a/fs/jbd/journal.c	Mon Nov 10 00:12:14 2003
+++ b/fs/jbd/journal.c	Fri Feb 27 20:36:04 2004
@@ -599,6 +599,7 @@
 		return NULL;
 
 	bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
+	memset(bh->b_data, 0, journal->j_blocksize);
 	bh->b_state |= (1 << BH_Dirty);
 	BUFFER_TRACE(bh, "return this buffer");
 	return journal_add_journal_head(bh);

Re: [SECURITY] CAN-2004-0075

[Michal Schmidt]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Greg KH wrote:
> On Wed, Apr 14, 2004 at 10:30:33PM +0200, Marc-Christian Petersen wrote:
>>Okay, now while we are at fixing security holes, is there any chance we 
>>can _finally_ get the attached patch in?
>>
>>The Vicam USB driver in all Linux Kernels 2.6 mainline does not use the 
>>copy_from_user function when copying data from userspace to kernel space, 
>>which crosses security boundaries and allows local users to cause a denial
>>of service.
>>
>>Already ACKed by Greg. Only complaint was inproper coding style which is done 
>>with attached patch ;)
> 
> 
> Eeek, I thought this one was already in the tree, very sorry about that.
> 
> I'm applying it now and will send it to Linus in a bit.
> 

The patch broke compilation with VICAM_DEBUG on.
There is also another copy_from_user missing in case VIDIOCSPICT.
I'm attaching a patch.

Michal Schmidt

[-- Attachment #2: vicam-ioctl.diff --]
[-- Type: text/plain, Size: 1125 bytes --]

--- linux-2.6.6-rc1/drivers/usb/media/vicam.c	2004-04-15 11:18:18.000000000 +0200
+++ linux-2.6.6-rc1-mich/drivers/usb/media/vicam.c	2004-04-15 11:50:02.791604312 +0200
@@ -612,15 +612,20 @@ vicam_ioctl(struct inode *inode, struct 
 
 	case VIDIOCSPICT:
 		{
-			struct video_picture *vp = (struct video_picture *) arg;
-
-			DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp->depth,
-			    vp->palette);
+			struct video_picture vp;
+			
+			if (copy_from_user(&vp, arg, sizeof(vp))) {
+				retval = -EFAULT;
+				break;
+			}
+			
+			DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp.depth,
+			    vp.palette);
 
-			cam->gain = vp->brightness >> 8;
+			cam->gain = vp.brightness >> 8;
 
-			if (vp->depth != 24
-			    || vp->palette != VIDEO_PALETTE_RGB24)
+			if (vp.depth != 24
+			    || vp.palette != VIDEO_PALETTE_RGB24)
 				retval = -EINVAL;
 
 			break;
@@ -660,7 +665,7 @@ vicam_ioctl(struct inode *inode, struct 
 				break;
 			}
 
-			DBG("VIDIOCSWIN %d x %d\n", vw->width, vw->height);
+			DBG("VIDIOCSWIN %d x %d\n", vw.width, vw.height);
 			
 			if ( vw.width != 320 || vw.height != 240 )
 				retval = -EFAULT;

Re: [SECURITY] CAN-2004-0177 (was: Re: [SECURITY] CAN-2004-0075)

[Stephen C. Tweedie]

Hi,

On Thu, 2004-04-15 at 00:35, Marc-Christian Petersen wrote:

> > Okay, now while we are at fixing security holes, is there any chance we
> > can get the attached patch in?
> 
> Okay, we are at it, so what's about the attached one too? ;)
> 
> In WOLK for some time too. I am not 100% sure if this is correct, but I think 
> it is. Andrew? Stephen?

Looks OK to me.  I'll see if I can detect any performance cost from it,
but it's unlikely to be significant even if it's measurable.

--Stephen