15 probable security holes in 2.4.5-ac8

15 probable security holes in 2.4.5-ac8

Post by Dawson Engle » Sun, 10 Jun 2001 06:40:09



Hi All,

here are 15 probable security holes where user input (e.g. data from
copy_from_user, get_user, etc) is:

        1. passed as a length argument to copy_*user (or passed to a
        function that does so),  OR

        2. is used as an array index.  

The main difference between this and past checkers is that we do
"inherited" tainting: if a structure is copied in from user space,
we recursively mark all of it's fields as tainted as well.  I'm still
fussing with the extension, so hopefully find a bunch more errors after
this batch.

You can look at this checker as essentially tracking whether the
information from an untrusted source (e.g., from copy_from_user) can reach
a trusting sink (e.g., an array index).  The main limiting factor on its
effectiveness is that we have a very incomplete notion of trusting sinks.
If anyone has suggestions for other general places where it's dangerous
to consume bad data, please send me an email.

Here's the location summary:

#  Total                                  = 15
# BUGs  |       File Name
3       |       drivers/char/drm/mga_state.c
2       |       drivers/scsi/megaraid.c
2       |       drivers/char/drm/i810_dma.c
2       |       drivers/char/raw.c
1       |       drivers/usb/usbvideo.c
1       |       drivers/usb/se401.c
1       |       drivers/net/hamradio/scc.c
1       |       drivers/char/moxa.c
1       |       drivers/media/video/zr36120.c
1       |       drivers/media/video/zr36067.c

Dawson
---------------------------------------------------------
[BUG]  dataxferlen is never checked.
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/scsi/megaraid.c:4108:megadev_ioctl: ERROR:RANGE:3825:4108: Using user length "dataxferlen" as argument to "copy_from_user" [type=LOCAL] [state = any_check] set by 'copy_from_user':3825 [val=28300]
        ret = verify_area (VERIFY_WRITE, (char *) arg, sizeof (struct uioctl_t));

        if (ret)
                return ret;

Start --->
        if(copy_from_user (&ioc, (char *) arg, sizeof (struct uioctl_t)))

        ... DELETED 277 lines ...

                        if (inlen) {
                                if (ioc.mbox[0] == MEGA_MBOXCMD_PASSTHRU) {
                                        /* copyin the user data */
                                        copy_from_user (kphysaddr,
                                                        uaddr,
Error --->
                                                        ioc.pthru.dataxferlen);
                                } else {
                                        copy_from_user (kphysaddr,
                                                        uaddr, inlen);
---------------------------------------------------------
[BUG] symetry.
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/scsi/megaraid.c:4167:megadev_ioctl: ERROR:RANGE:3825:4167: Using user length "dataxferlen" as argument to "copy_to_user" [type=LOCAL] [state = any_check] set by 'copy_from_user':3825 [val=34200]
        ret = verify_area (VERIFY_WRITE, (char *) arg, sizeof (struct uioctl_t));

        if (ret)
                return ret;

Start --->
        if(copy_from_user (&ioc, (char *) arg, sizeof (struct uioctl_t)))

        ... DELETED 336 lines ...

                down (&mimd_ioctl_sem);

                if (!scsicmd->result && outlen) {
                        if (ioc.mbox[0] == MEGA_MBOXCMD_PASSTHRU) {
                                copy_to_user (uaddr,
Error --->
                                              kphysaddr, ioc.pthru.dataxferlen);
                        } else {
                                copy_to_user (uaddr, kphysaddr, outlen);
                        }
---------------------------------------------------------
[BUG]
static int moxaload320b(int cardno, unsigned char * tmp, int len)
{
        unsigned long baseAddr;
        int i;
        if(copy_from_user(moxaBuff, tmp, len))
                return -EFAULT;
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/moxa.c:1730:MoxaDriverIoctl: ERROR:RANGE:1728:1730: Using user length "len" as argument to "moxaload320b" [type=GLOBAL] [state = tainted] set by 'copy_from_user':1728 [val=200]
        case MOXA_FIND_BOARD:
                if(copy_from_user(&dltmp, (void *)arg, sizeof(struct dl_str)))
                        return -EFAULT;
                return moxafindcard(dltmp.cardno);
        case MOXA_LOAD_C320B:
Start --->
                if(copy_from_user(&dltmp, (void *)arg, sizeof(struct dl_str)))
                        return -EFAULT;
Error --->
                moxaload320b(dltmp.cardno, dltmp.buf, dltmp.len);
                return (0);
        case MOXA_LOAD_CODE:
                if(copy_from_user(&dltmp, (void *)arg, sizeof(struct dl_str)))
---------------------------------------------------------
[BUG]  d.idx is an int: < 0 = bad news.
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/i810_dma.c:1413:i810_copybuf: ERROR:RANGE:1409:1413: Using user length "idx"as an array index for "buflist" set by 'copy_from_user':1409 [val=400]
        if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
                DRM_ERROR("i810_dma called without lock held\n");
                return -EINVAL;
        }

Start --->
        if (copy_from_user(&d, (drm_i810_copy_t *)arg, sizeof(d)))
                return -EFAULT;

        if(d.idx > dma->buf_count) return -EINVAL;
Error --->
        buf = dma->buflist[ d.idx ];
        buf_priv = buf->dev_private;
        if (buf_priv->currently_mapped != I810_BUF_MAPPED) return -EPERM;

---------------------------------------------------------
[BUG]  ouch.  (routine also uses it as an index)
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/usb/se401.c:1290:se401_ioctl: ERROR:RANGE:1286:1290: Using user length "frame"as an array index for "frame" set by 'copy_from_user':1286 [val=400]
        }
        case VIDIOCSYNC:
        {
                int frame, ret=0;

Start --->
                if (copy_from_user((void *)&frame, arg, sizeof(int)))
                        return -EFAULT;

                ret=se401_newframe(se401, frame);
Error --->
                se401->frame[frame].grabstate=FRAME_UNUSED;
                return ret;
        }
        case VIDIOCGFBUF:
---------------------------------------------------------
[BUG]  ouch x 2: no check either way.
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/mga_state.c:835:mga_iload: ERROR:RANGE:827:835: Using user length "idx"as an array index for "buflist" set by 'copy_from_user':827 [val=800]
        drm_buf_t *buf;
        drm_mga_buf_priv_t *buf_priv;
        drm_mga_iload_t iload;
        unsigned long bus_address;

Start --->
        if (copy_from_user(&iload, (drm_mga_iload_t *) arg, sizeof(iload)))
                return -EFAULT;

        if (!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
                DRM_ERROR("mga_iload called without lock held\n");
                return -EINVAL;
        }

Error --->
        buf = dma->buflist[iload.idx];
        buf_priv = buf->dev_private;
        bus_address = buf->bus_address;

---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/mga_state.c:923:mga_indices: ERROR:RANGE:915:923: Using user length "idx"as an array index for "buflist" set by 'copy_from_user':915 [val=800]
        drm_buf_t *buf;
        drm_mga_buf_priv_t *buf_priv;
        drm_mga_indices_t indices;

        if (copy_from_user(&indices,
Start --->
                           (drm_mga_indices_t *)arg, sizeof(indices)))
                return -EFAULT;

        if (!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
                DRM_ERROR("mga_indices called without lock held\n");
                return -EINVAL;
        }

Error --->
        buf = dma->buflist[indices.idx];
        buf_priv = buf->dev_private;

        buf_priv->discard = indices.discard;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/mga_state.c:877:mga_vertex: ERROR:RANGE:869:877: Using user length "idx"as an array index for "buflist" set by 'copy_from_user':869 [val=800]
        drm_device_dma_t *dma = dev->dma;
        drm_buf_t *buf;
        drm_mga_buf_priv_t *buf_priv;
        drm_mga_vertex_t vertex;

Start --->
        if (copy_from_user(&vertex, (drm_mga_vertex_t *) arg, sizeof(vertex)))
                return -EFAULT;

        if (!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
                DRM_ERROR("mga_vertex called without lock held\n");
                return -EINVAL;
        }

Error --->
        buf = dma->buflist[vertex.idx];
        buf_priv = buf->dev_private;

        buf->used = vertex.used;
---------------------------------------------------------
[BUG] (but i'm not sure whey we're missing the initial irq).
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/net/hamradio/scc.c:1772:scc_net_ioctl: ERROR:RANGE:1762:1772: Using user length "irq"as an array index for "Ivec" set by 'copy_from_user':1762 [val=1000]
                        if (!arg) return -EFAULT;

                        if (Nchips >= SCC_MAXCHIPS)
                                return -EINVAL;

Start --->
                        if (copy_from_user(&hwcfg, arg, sizeof(hwcfg)))
                                return -EFAULT;

                        if (hwcfg.irq == 2) hwcfg.irq = 9;

                        if (!Ivec[hwcfg.irq].used && hwcfg.irq)
                        {
                                if (request_irq(hwcfg.irq, scc_isr, SA_INTERRUPT, "AX.25 SCC", NULL))
                                        printk(KERN_WARNING "z8530drv: warning, cannot get IRQ %d\n", hwcfg.irq);
                                else
Error --->
                                        Ivec[hwcfg.irq].used = 1;
                        }

                        if (hwcfg.vector_latch) {
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/i810_dma.c:1291:i810_dma_vertex: ERROR:RANGE:1278:1291: Using user length "idx"as an array index for "buflist" set by 'copy_from_user':1278 [val=1300]
        u32 *hw_status = (u32 *)dev_priv->hw_status_page;
        drm_i810_sarea_t *sarea_priv = (drm_i810_sarea_t *)
                                        dev_priv->sarea_priv;
        drm_i810_vertex_t vertex;

Start --->
        if (copy_from_user(&vertex, (drm_i810_vertex_t *)arg, sizeof(vertex)))
                return -EFAULT;

        if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
                DRM_ERROR("i810_dma_vertex called without lock held\n");
                return -EINVAL;
        }

        DRM_DEBUG("i810 dma vertex, idx %d used %d discard %d\n",
                  vertex.idx, vertex.used, vertex.discard);

        i810_dma_dispatch_vertex( dev,
                                  dma->buflist[ vertex.idx ],
Error --->
                                  vertex.discard, vertex.used );

        atomic_add(vertex.used, &dma->total_bytes);
        atomic_inc(&dma->total_dmas);
---------------------------------------------------------
[BUG]  channel is an integer.
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/media/video/zr36120.c:1032:zoran_ioctl: ERROR:RANGE:1010:1032: Using user length "channel"as an array index for "video_mux" set by 'copy_from_user':1010 [val=2200]

         case VIDIOCGCHAN:
         {
                struct video_channel v;
                int mux;
Start --->
                if (copy_from_user(&v, arg,sizeof(v)))

        ... DELETED 16 lines ...

                /* too many inputs? no decoder -> no channels */
                if (!ztv->have_decoder || v.channel >= ztv->card->video_inputs)
                        return -EINVAL;

                /* now determine the name of the channel */
Error --->
                mux = ztv->card->video_mux[v.channel];
                if (mux & IS_TUNER) {
                        /* lets assume only one tuner, yes? */
                        strcpy(v.name,"Television");
---------------------------------------------------------
[BUG]  usbvideo_GetFrame can fail, but result is not checked before index.
/u2/engler/mc/oses/linux/2.4.5-ac8/drivers/usb/usbvideo.c:1596:usbvideo_v4l_ioctl: ERROR:RANGE:1572:1596: Using user length "frameNum"as an array index for ...

read more »

 
 
 

15 probable security holes in 2.4.5-ac8

Post by Oliver Xymoro » Sun, 10 Jun 2001 08:30:11



> You can look at this checker as essentially tracking whether the
> information from an untrusted source (e.g., from copy_from_user) can reach
> a trusting sink (e.g., an array index).  The main limiting factor on its
> effectiveness is that we have a very incomplete notion of trusting sinks.
> If anyone has suggestions for other general places where it's dangerous
> to consume bad data, please send me an email.

I wrote something similar to this for userspace (via ld preload). Most of
my checks followed strings around and made sure they were length checked
so as to avoid stack overflows, but I also checked args to open, etc..

In your case, basically all destinations are trusting sinks at some level:
userspace gives you data to put it somewhere. You might want to instead
check that data is passing through functions that 'detaint', by checking
capabilities, etc. I bet that you'll find that something like 90% of code
paths are covered by a few common security checks. And that most of the
remainder could be rewritten to be.

--
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

1. 15 probable security holes in 2.4.5-ac8


Subject: Re: [CHECKER] 15 probable security holes in 2.4.5-ac8
Date: Mon, 11 Jun 2001 15:45:07 +0200 (CEST)

Nah, just a misconception (NB: the whole scc driver initialization is crap
anyway -- but that part was written before we even had procfs; the next
version will use procfs, but I'm not quite convinced that my current
approach for the rewrite is correct. Fact is that the driver has to support
far too many different parameters). The next version will also use
the ISR of your z85230 HDLC driver, the z8530 seems to occasionally
overwrite it's interrupt vector register with new status information
before the old one was read.

They won't assign IRQs above 15 for ISA cards, will they?

I gravely hope that nobody gets the idea to design a PCI card
for the Z8530 without bus master DMA...

Granted. But I've no reports that anyone actually tried that,
especially as the (unmodified) driver is only useful for packet radio
purposes.

How? ;-)

73,
--
Joerg Reuter DL1BKE                             http://yaina.de/jreuter
And I make my way to where the warm scent of soil fills the evening air.
Everything is waiting quietly out there....                 (Anne Clark)

  application_pgp-signature_part
< 1K Download

2. Betcha can't figger thisun out!

3. Setup dependant probable security hole

4. Can not start Apache 1.3.17 on SGI system get Error=3

5. two probable security holes

6. dip

7. 52 probable security holes in 2.4.6 and 2.4.6-ac2

8. EEPROM recovery

9. BIOS Int 15-E820, memory holes, and high_memory

10. How can I safely read/write on the 15/16MB memory hole?

11. Enabling the 15-16 MB memory hole?

12. 15/16 MB memory hole

13. Linux with 15~16M memory hole?