Thursday, 25 January 2018

NetBSD kernel wscons IOCTL vulnerable bug class

Dr Silvio Cesare


In this blog post I discuss a vulnerable bug class that exists in the NetBSD kernel based on an incorrect coding style that has an integer overflow during input validation. I find 17 vulnerabilities and variants. I write a coccinelle script to automatically detect 16 instances of the integer overflow bugs with an additional 5 false positives. Furthermore, I manually find another bug that isn't an integer overflow, but in fact is code that has no input validation at all.

1. Introduction

I discovered this bug class during the InfoSect public code review session we ran looking specifically at the NetBSD kernel. I found a couple of these bugs and then after the session was complete, I went back and realised the same bug was scattered in other drivers. In total, 17 instances of this vulnerability and its variants were discovered.

 In all fairness, I came across this bug class during my kernel audits in 2002 and most instances were patched. It just seems there are more bugs now in NetBSD while OpenBSD and FreeBSD have practically eliminated them.

See slide 41 in http://www.blackhat.com/presentations/bh-usa-03/bh-us-03-cesare.pdf for exactly the same bug (class) 16 years ago.

 The format of the this blog post is as follows:

  1. Introduction
  2. Example of the Bug Class
  3. How to Fix
  4. How to Detect Automatically with Coccinelle
  5. More Bugs
  6. Conclusion
These source files had bugs
  1. ./dev/tc/tfb.c
  2. ./dev/ic/bt485.c
  3. ./dev/pci/radeonfb.c
  4. ./dev/ic/sti.c
  5. ./dev/sbus/tcx.c
  6. ./dev/tc/mfb.c
  7. ./dev/tc/sfb.c
  8. ./dev/tc/stic.c
  9. ./dev/tc/cfb.c
  10. ./dev/tc/xcfb.c
  11. ./dev/tc/sfbplus.c
  12. ./arch/arm/allwinner/awin_debe.c
  13. ./arch/arm/iomd/vidcvideo.c
  14. ./arch/pmax/ibus/pm.c
  15. ./dev/ic/igfsb.c
  16. ./dev/ic/bt463.c
  17. ./arch/luna68k/dev/lunafb.c
Reporting of the bugs was easy. In less than a week from reporting the specific instances of each bug, patches were committed into the mainline kernel. Thanks to Luke Mewburn from NetBSD for coming to the code review session at InfoSect and coordinating with the NetBSD security team.

The patches to fix these issues are in NetBSD:
https://mail-index.netbsd.org/source-changes/2018/01/24/msg091428.html

2. Example of the Bug Class

I'll give an example.

 ./dev/wscons/wsconsio.h

struct wsdisplay_cmap { u_int index; /* first element (0 origin) */ u_int count; /* number of elements */ u_char *red; /* red color map elements */ u_char *green; /* green color map elements */ u_char *blue; /* blue color map elements */ };

...

 struct wsdisplay_cursor { u_int which; /* values to get/set */ #define WSDISPLAY_CURSOR_DOCUR 0x01 /* get/set enable */ #define WSDISPLAY_CURSOR_DOPOS 0x02 /* get/set pos */ #define WSDISPLAY_CURSOR_DOHOT 0x04 /* get/set hot spot */ #define WSDISPLAY_CURSOR_DOCMAP 0x08 /* get/set cmap */ #define WSDISPLAY_CURSOR_DOSHAPE 0x10 /* get/set img/mask */ #define WSDISPLAY_CURSOR_DOALL 0x1f /* all of the above */ u_int enable; /* enable/disable */ struct wsdisplay_curpos pos; /* position */ struct wsdisplay_curpos hot; /* hot spot */ struct wsdisplay_cmap cmap; /* color map info */ struct wsdisplay_curpos size; /* bit map size */ u_char *image; /* image data */ u_char *mask; /* mask data */ };


./dev/tc/tfb.c
static int set_cursor(struct tfb_softc *sc, struct wsdisplay_cursor *p) { #define cc (&sc->sc_cursor) u_int v, index = 0, count = 0, icount = 0; uint8_t r[2], g[2], b[2], image[512], mask[512]; int error, s; v = p->which; if (v & WSDISPLAY_CURSOR_DOCMAP) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) +++ integer overflow return (EINVAL); error = copyin(p->cmap.red, &r[index], count); if (error) return error; error = copyin(p->cmap.green, &g[index], count); if (error) return error; error = copyin(p->cmap.blue, &b[index], count); if (error) return error;

 set_cursor is called via an IOCTL. index and count are under control by the user calling the IOCTL. The input validation should be simple - neither index and count or the sum of index and count should be out of range. In the code above, we have a simple integer overflow when count is close to UINT_MAX. The input validation fails leading to a large size to the copyin API. This will lead to copying as much memory as permissible from userland into the kernel.

3. How to Fix?

We'll look at OpenBSD.

 int
rfx_putcmap(struct rfx_cmap *cm, struct wsdisplay_cmap *rcm)
{
        u_int index = rcm->index, count = rcm->count;
        u_int8_t red[256], green[256], blue[256];
        int error;

         if (index >= 256 || count > 256 - index)
                return (EINVAL);

         if ((error = copyin(rcm->red, red, count)) != 0)
                return (error);
        if ((error = copyin(rcm->green, green, count)) != 0)
                return (error);
        if ((error = copyin(rcm->blue, blue, count)) != 0)

                 return (error);

The above code correctly handles integer overflows and will enforce bounds checking. It looks like FreeBSD and OpenBSD have effectively eliminated this class of integer overflows from the given wscons code.

4. How to Detect Automatically with Coccinelle

This bug pattern can be templated into a coccinelle script.

 $ cat wscons.cocci
@@
expression E1, E2, E3, E4;
@@

 (
* E1 > E2 || E1 + E3 > E4
|
* E1 >= E2 || E1 + E3 > E4
|
* E1 >= E2 || E1 + E3 >= E4
|
* E1 > E2 || E1 + E3 >= E4
|
* E1 > E2 || (E1 + E3) > E4
|
* E1 >= E2 || (E1 + E3) > E4
|
* E1 >= E2 || (E1 + E3) >= E4
|
* E1 > E2 || (E1 + E3) >= E4
|
* E1 > E2 || E3 + E1 > E4
|
* E1 >= E2 || E3 + E1 > E4
|
* E1 >= E2 || E3 + E1 >= E4
|
* E1 > E2 || E3 + E1 >= E4
|
* E1 > E2 || (E3 + E1) > E4
|
* E1 >= E2 || (E3 + E1) > E4
|
* E1 >= E2 || (E3 + E1) >= E4
|
* E1 > E2 || (E3 + E1) >= E4
|
* E1 + E3 > E4 || E1 > E2
|
* E1 + E3 > E4 || E1 >= E2
|
* E1 + E3 >= E4 || E1 >= E2
|
* E1 + E3 >= E4 || E1 > E2
|
* (E1 + E3) > E4 || E1 > E2
|
* (E1 + E3) > E4 || E1 >= E2
|
* (E1 + E3) >= E4 || E1 >= E2
|
* (E1 + E3) >= E4 || E1 > E2
|
* E3 + E1 > E4 || E1 > E2
|
* E3 + E1 > E4 || E1 >= E2
|
* E3 + E1 >= E4 || E1 >= E2
|
* E3 + E1 >= E4 || E1 > E2
|
* (E3 + E1) > E4 || E1 > E2
|
* (E3 + E1) > E4 || E1 >= E2
|
* (E3 + E1) >= E4 || E1 >= E2
|
* (E3 + E1) >= E4 || E1 > E2

)

To run coccinelle

 $ spatch --sp-file wscons.cocci --dir NetBSD-7.1/

This script detects 16 bugs that account for all discovered integer overflows in this bug class. 5 reports are false positive. This represents a usable script that doesn't take long to check the results.

5. More Bugs

Lets look at more NetBSD code.

./dev/ic/bt485.c
int bt485_set_cursor(struct ramdac_cookie *rc, struct wsdisplay_cursor *cursorp) { struct bt485data *data = (struct bt485data *)rc; u_int count = 0, icount = 0, index = 0, v; char r[2], g[2], b[2], image[512], mask[512]; int s, error; v = cursorp->which; /* * For DOCMAP and DOSHAPE, copy in the new data * before we do anything that we can't recover from. */ if (v & WSDISPLAY_CURSOR_DOCMAP) { if (cursorp->cmap.index > 2 || (cursorp->cmap.index + cursorp->cmap.count) > 2) return (EINVAL); +++ integer overflow with index and count count = cursorp->cmap.count; index = cursorp->cmap.index; error = copyin(cursorp->cmap.red, &r[index], count); +++ buffer overflow copy from user space if (error)

And another..

 ./dev/pci/radeonfb.c

 static int radeonfb_set_cursor(struct radeonfb_display *dp, struct wsdisplay_cursor *wc) { unsigned flags; uint8_t r[2], g[2], b[2]; unsigned index, count; int i, err; int pitch, size; struct radeonfb_cursor nc; flags = wc->which; /* copy old values */ nc = dp->rd_cursor; if (flags & WSDISPLAY_CURSOR_DOCMAP) { index = wc->cmap.index; count = wc->cmap.count; if (index >= 2 || (index + count) > 2) return EINVAL;


And more..

./dev/ic/sti.c
/* * wsdisplay accessops */ int sti_ioctl(void *v, void *vs, u_long cmd, void *data, int flag, struct lwp *l) { struct sti_screen *scr = (struct sti_screen *)v; struct sti_rom *rom = scr->scr_rom; struct wsdisplay_fbinfo *wdf; struct wsdisplay_cmap *cmapp; u_int mode, idx, count; int i, ret; ... case WSDISPLAYIO_GETCMAP: if (rom->scment == NULL) return ENOTTY; cmapp = (struct wsdisplay_cmap *)data; idx = cmapp->index; count = cmapp->count; if (idx >= STI_NCMAP || idx + count > STI_NCMAP) +++ integer overflow with idx + count return EINVAL; if ((ret = copyout(&scr->scr_rcmap[idx], cmapp->red, count))) break; if ((ret = copyout(&scr->scr_gcmap[idx], cmapp->green, count))) break; if ((ret = copyout(&scr->scr_bcmap[idx], cmapp->blue, count))) break; break;

And now a bug that doesn't do any input validation. This isn't really the same bug class, but I've included it anyway.

./dev/sbus/tcx.c
int tcxioctl(dev_t dev, u_long cmd, void *data, int flags, struct lwp *l) { ... case FBIOGETCMAP: #define p ((struct fbcmap *)data) if (copyout(&sc->sc_cmap_red[p->index], p->red, p->count) != 0) return EINVAL; if (copyout(&sc->sc_cmap_green[p->index], p->green, p->count)  != 0) return EINVAL; if (copyout(&sc->sc_cmap_blue[p->index], p->blue, p->count)  != 0) return EINVAL; return 0; case FBIOPUTCMAP: /* copy to software map */ if (copyin(p->red, &sc->sc_cmap_red[p->index], p->count) != 0) return EINVAL; if (copyin(p->green, &sc->sc_cmap_green[p->index], p->count)  != 0) return EINVAL; +++ no input validation on p->count in any of the above

Back to the original bug class of an integer overflow..

./dev/tc/mfb.c
static int set_cursor(struct mfb_softc *sc, struct wsdisplay_cursor *p) { #define cc (&sc->sc_cursor) u_int v, count = 0, icount = 0, index = 0; uint64_t image[CURSOR_MAX_SIZE]; uint64_t mask[CURSOR_MAX_SIZE]; uint8_t color[6]; int error, s; v = p->which; if (v & WSDISPLAY_CURSOR_DOCMAP) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) +++ integer overflow return (EINVAL); error = copyin(p->cmap.red, &color[index], count); if (error) return error; }

Lets keep going..

./dev/tc/sfb.c
static int set_cursor(struct sfb_softc *sc, struct wsdisplay_cursor *p) { #define cc (&sc->sc_cursor) u_int v, index = 0, count = 0, icount = 0; uint8_t r[2], g[2], b[2], image[512], mask[512]; int error, s; v = p->which; if (v & WSDISPLAY_CURSOR_DOCMAP) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) +++ integer overflow return (EINVAL); error = copyin(p->cmap.red, &r[index], count); if (error) return error; error = copyin(p->cmap.green, &g[index], count); if (error) return error;

And still more..

./dev/tc/stic.c
static int stic_set_cursor(struct stic_info *si, struct wsdisplay_cursor *p) { #define cc (&si->si_cursor) u_int v, index = 0, count = 0, icount = 0; struct stic_screen *ss; uint8_t r[2], g[2], b[2], image[512], mask[512]; int s, error; v = p->which; ss = si->si_curscreen; if ((v & WSDISPLAY_CURSOR_DOCMAP) != 0) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) +++ integer overflow return (EINVAL); error = copyin(p->cmap.red, &r[index], count); if (error) return error; error = copyin(p->cmap.green, &g[index], count); if (error)

More..

./dev/tc/cfb.c
static int set_cursor(struct cfb_softc *sc, struct wsdisplay_cursor *p) { #define cc (&sc->sc_cursor) u_int v, index = 0, count = 0, icount = 0; uint8_t r[2], g[2], b[2], image[512], mask[512]; int error, s; v = p->which; if (v & WSDISPLAY_CURSOR_DOCMAP) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) +++ integer overflow return (EINVAL); error = copyin(p->cmap.red, &r[index], count); if (error) return error; error = copyin(p->cmap.green, &g[index], count); if (error) return error; error = copyin(p->cmap.blue, &b[index], count);

And..

./dev/tc/xcfb.c
static int set_cursor(struct xcfb_softc *sc, struct wsdisplay_cursor *p) { #define cc (&sc->sc_cursor) u_int v, index = 0, count = 0, icount = 0; uint8_t r[2], g[2], b[2], image[512], mask[512]; int error; v = p->which; if (v & WSDISPLAY_CURSOR_DOCMAP) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || index + count > 2) +++ integer overflow return (EINVAL); error = copyin(p->cmap.red, &r[index], count); if (error) return error; error = copyin(p->cmap.green, &g[index], count); if (error) return error;

And..

./dev/tc/sfbplus.c
static int set_cursor(struct sfbp_softc *sc, struct wsdisplay_cursor *p) { #define cc (&sc->sc_cursor) u_int v, index = 0, count = 0, icount = 0; uint8_t r[2], g[2], b[2], image[512], mask[512]; int error, s; v = p->which; if (v & WSDISPLAY_CURSOR_DOCMAP) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) return (EINVAL); ++ integer overflow with index+count error = copyin(p->cmap.red, &r[index], count); if (error) return error; error = copyin(p->cmap.green, &g[index], count); if (error)

And..

./arch/arm/allwinner/awin_debe.c
static int awin_debe_set_cursor(struct awin_debe_softc *sc, struct wsdisplay_cursor *cur) { uint32_t val; uint8_t r[4], g[4], b[4]; u_int index, count, shift, off, pcnt; int i, j, idx, error; uint8_t mask; ... if (cur->which & WSDISPLAY_CURSOR_DOCMAP) { index = cur->cmap.index; count = cur->cmap.count; if (index >= 2 || (index + count) > 2) return EINVAL; +++ integer overflow error = copyin(cur->cmap.red, &r[index], count); if (error) return error;

And..

./arch/arm/iomd/vidcvideo.c
static int set_cmap(struct vidcvideo_softc *sc, struct wsdisplay_cmap *p) { struct fb_devconfig *dc = sc->sc_dc; struct hwcmap256 cmap; u_int index = p->index, count = p->count; int error; if (index >= CMAP_SIZE || (index + count) > CMAP_SIZE) return EINVAL; +++ integer overflow error = copyin(p->red, &cmap.r[index], count); if (error) return error; error = copyin(p->green, &cmap.g[index], count); if (error) return error; error = copyin(p->blue, &cmap.b[index], count); if (error) return error;

Several bugs now..

./arch/pmax/ibus/pm.c
int pm_get_cmap(struct pm_softc *sc, struct wsdisplay_cmap *p) { u_int index, count; int rv; index = p->index; count = p->count; if (index >= sc->sc_cmap_size || (index + count) > sc->sc_cmap_size) return (EINVAL); +++ integer overflow if ((rv = copyout(&sc->sc_cmap.r[index], p->red, count)) != 0) return (rv); if ((rv = copyout(&sc->sc_cmap.g[index], p->green, count)) != 0) return (rv); return (copyout(&sc->sc_cmap.b[index], p->blue, count)); ... int pm_set_cmap(struct pm_softc *sc, struct wsdisplay_cmap *p) { u_int index, count; int rv; index = p->index; count = p->count; if (index >= sc->sc_cmap_size || (index + count) > sc->sc_cmap_size) return (EINVAL); +++ integer overflow if ((rv = copyin(p->red, &sc->sc_cmap.r[index], count)) != 0) return (rv); if ((rv = copyin(p->green, &sc->sc_cmap.g[index], count)) != 0) return (rv); if ((rv = copyin(p->blue, &sc->sc_cmap.b[index], count)) != 0) return (rv); sc->sc_changed |= WSDISPLAY_CMAP_DOLUT; return (0); } ... int pm_set_cursor(struct pm_softc *sc, struct wsdisplay_cursor *p) { u_int v, index, count; struct hwcursor64 *cc; int rv; v = p->which; cc = &sc->sc_cursor; ... if ((v & WSDISPLAY_CURSOR_DOCMAP) != 0) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) return (EINVAL); +++ integer overflow rv = copyin(p->cmap.red, &cc->cc_color[index], count); if (rv != 0) return (rv); rv = copyin(p->cmap.green, &cc->cc_color[index + 2], count); if (rv != 0) return (rv); rv = copyin(p->cmap.blue, &cc->cc_color[index + 4], count); if (rv != 0) return (rv); }

And..

./dev/ic/igfsb.c
static void igsfb_update_cmap(struct igsfb_devconfig *dc, u_int index, u_int count) { bus_space_tag_t t; bus_space_handle_t h; u_int last, i; if (index >= IGS_CMAP_SIZE) return; last = index + count; +++ integer overflow if (last > IGS_CMAP_SIZE) last = IGS_CMAP_SIZE; ... static int igsfb_set_cursor(struct igsfb_devconfig *dc, const struct wsdisplay_cursor *p) { struct igs_hwcursor *cc; struct wsdisplay_curpos pos, hot; u_int v, index, count, icount, iwidth; uint8_t r[2], g[2], b[2], image[512], mask[512]; int error; cc = &dc->dc_cursor; v = p->which; index = count = icount = iwidth = 0; /* XXX: gcc */ pos.x = pos.y = 0; /* XXX: gcc */ /* copy in the new cursor colormap */ if (v & WSDISPLAY_CURSOR_DOCMAP) { index = p->cmap.index; count = p->cmap.count; if (index >= 2 || (index + count) > 2) +++ integer overflow return EINVAL; error = copyin(p->cmap.red, &r[index], count); if (error)

And more..

./dev/ic/bt463.c int bt463_check_curcmap(struct ramdac_cookie *rc, struct wsdisplay_cursor *cursorp) { struct bt463data *data = (struct bt463data *)rc; int count, index, error; if (cursorp->cmap.index > 2 || (cursorp->cmap.index + cursorp->cmap.count) > 2) return (EINVAL); +++ integer overflow count = cursorp->cmap.count; index = cursorp->cmap.index; error = copyin(cursorp->cmap.red, &data->tmpcurcmap_r[index], count); if (error) return error; error = copyin(cursorp->cmap.green, &data->tmpcurcmap_g[index], count); if (error) return error; error = copyin(cursorp->cmap.blue, &data->tmpcurcmap_b[index], count); if (error) return error; return (0);

And..

./arch/luna68k/dev/lunafb.c

static int
omsetcmap(struct omfb_softc *sc, struct wsdisplay_cmap *p)
{
        struct hwcmap cmap;
        u_int index = p->index, count = p->count;
        int cmsize, i, error;

         cmsize = sc->sc_dc->dc_cmsize;
        if (index >= cmsize || (index + count) > cmsize)
                return (EINVAL);

         error = copyin(p->red, &cmap.r[index], count);
        if (error)
                return error;
        error = copyin(p->green, &cmap.g[index], count);
        if (error)
                return error;
        error = copyin(p->blue, &cmap.b[index], count);
        if (error)

                return error;

6. Conclusion

In this blog post, I identified a bug class in the NetBSD kernel. I detected the presence of these bugs manually and automatically to reveal 17 unknown bugs.
at
Labels:

Exploiting the Lorex 2K Indoor Wifi at Pwn2Own Ireland

Introduction In October InfoSect participated in Pwn2Own Ireland 2024 and successfully exploited the Sonos Era 300 smart speaker and Lor...