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.

Popular posts from this blog

Empowering Women in Cybersecurity: InfoSect's 2024 Training Initiative

 InfoSect has always been committed to fostering diversity and inclusion within the cybersecurity industry, with a special focus on encouraging those who identify as women to excel in the technical streams. We're excited to announce that in 2024, we're taking our commitment to the next level. Following the resounding success of our 2022 initiative, we are proud to double our efforts and expand our training opportunities for women in cybersecurity. HackerChix: Uniting Women in Cybersecurity In 2017, InfoSect introduced HackerChix, a vibrant community that provides a platform for women to connect, support, and inspire one another in the world of cybersecurity. This initiative has been a constant presence at BSides Canberra every year.   Doubling Down on Education To empower more women with the knowledge and skills needed to excel in cybersecurity, InfoSect is thrilled to announce a significant expansion of its training offerings in 2024. Thanks to the generous sponsorshi
Read more

C++ Memory Corruption (std::string) - part 4

  Summary This is the next part of the C++ memory corruption series*. In this post, we'll look at corrupting the std:string object in Linux and see what exploitation primitives we can gain. * https://blog.infosectcbr.com.au/2020/08/c-memory-corruption-part-1.html * https://blog.infosectcbr.com.au/2022/01/c-memory-corruption-stdvector-part-2.html  *  https://blog.infosectcbr.com.au/2022/03/c-memory-corruption-stdlist-part-3.html Author: Dr Silvio Cesare Introduction C++ is a common language for memory corruption. However, there is much more literature on exploiting C programs and little on C++ programs. C++ presents new classes, objects, and data structures which can all be effectively used for building exploitation primitives. In this post, we'll look at corrupting the std::string class and see what specific primitives we can obtain. std::string We note that the object stored in memory for a basic string consists firstly of the backing pointer to the string contents. Se
Read more

Pointer Compression in V8

Image
Syed Faraz Abrar @farazsth98   Summary In this blog post, I will provide some details on how the Chromium developers implemented pointer compression in V8. I will also talk about what this means from an exploit development perspective. Introduction I’ve been an intern at InfoSect for the past couple of weeks now, and in this time, I’ve had to do a bunch of security related research into both V8 and Spidermonkey. One of the things that I spent a short amount of my time on was pointer compression in V8. I hadn’t heard of the term at all until Bruno Keith ( @bkth_ ) mentioned it on twitter some time in December last year. The V8 developers also made a blog post to celebrate V8 v8.0 where they mentioned that implementing pointer compression had allowed them to save up to 40% in usage of heap memory! That’s a big improvement, so let’s take a look at what pointer compression is and what it means from an exploit developer’s perspective. Disclaimer This is not intended to
Read more