Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 29 Jan 2016 21:51:31 +0000
From:      bugzilla-noreply@freebsd.org
To:        freebsd-bugs@FreeBSD.org
Subject:   [Bug 206749] Lack of checks on values in ELF headers in kernel linker
Message-ID:  <bug-206749-8@https.bugs.freebsd.org/bugzilla/>
index | next in thread | raw e-mail 

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206749

            Bug ID: 206749
           Summary: Lack of checks on values in ELF headers in kernel
                    linker
           Product: Base System
           Version: 11.0-CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: kern
          Assignee: freebsd-bugs@FreeBSD.org
          Reporter: cturt@hardenedbsd.org

There are a lack of checks performed on the `e_shnum` and `e_shentsize` values
read from input files in the kernel's ELF handling code.

For example, in `link_elf_ctf_get`:

static int
link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
{
        ...
        int nbytes;

        ...

        /* Allocate memory for the FLF header. */
        if ((hdr = malloc(sizeof(*hdr), M_LINKER, M_WAITOK)) == NULL) {
                error = ENOMEM;
                goto out;
        }

        /* Read the ELF header. */
        if ((error = vn_rdwr(UIO_READ, nd.ni_vp, hdr, sizeof(*hdr),
            0, UIO_SYSSPACE, IO_NODELOCKED, td->td_ucred, NOCRED, &resid,
            td)) != 0)
                goto out;

        /* Sanity check. */
        if (!IS_ELF(*hdr)) {
                error = ENOEXEC;
                goto out;
        }

        nbytes = hdr->e_shnum * hdr->e_shentsize;
        if (nbytes == 0 || hdr->e_shoff == 0 ||
            hdr->e_shentsize != sizeof(Elf_Shdr)) {
                error = ENOEXEC;
                goto out;
        }

        /* Allocate memory for all the section headers */
        if ((shdr = malloc(nbytes, M_LINKER, M_WAITOK)) == NULL) {
                error = ENOMEM;
                goto out;
        }

        ...
}

The `e_shnum` and `e_shentsize` fields are declared as `Elf32_Half` (a
`typedef` for `uint16_t`) in the `Elf_Ehdr` structure (`sys/sys/elf32.h`):

           typedef struct {
                   unsigned char   e_ident[EI_NIDENT];
                   Elf32_Half      e_type;
                   Elf32_Half      e_machine;
                   Elf32_Word      e_version;
                   Elf32_Addr      e_entry;
                   Elf32_Off       e_phoff;
                   Elf32_Off       e_shoff;
                   Elf32_Word      e_flags;
                   Elf32_Half      e_ehsize;
                   Elf32_Half      e_phentsize;
                   Elf32_Half      e_phnum;
                   Elf32_Half      e_shentsize;
                   Elf32_Half      e_shnum;
                   Elf32_Half      e_shstrndx;
           } Elf32_Ehdr;

So consider if `0xffff` and `0x8001` are supplied for `e_shentsize` and
`e_shnum`:

    nbytes = hdr->e_shnum * hdr->e_shentsize;
    nbytes = 0xffff * 0x8001 = 0x80007FFF;

Since `nbytes` is `signed`, this will of course wrap around to `-0x7FFF8001`.

When `-0x7FFF8001` is passed to `malloc` it will be converted to an unsigned
64bit integer, giving an oversized allocation of `0xFFFFFFFF80007FFF`.

This poses no immediate security threat since the result from `malloc` is
checked, and will return `ENOMEM` for an overflown size.

This bug only applies to the `e_shnum` and `e_shentsize` members; the `e_phnum`
and `e_phentsize` members are correctly checked multiple times before use:

        if (!((hdr->e_phentsize == sizeof(Elf_Phdr)) &&
              (hdr->e_phoff + hdr->e_phnum*sizeof(Elf_Phdr) <= PAGE_SIZE) &&
              (hdr->e_phoff + hdr->e_phnum*sizeof(Elf_Phdr) <= nbytes)))
                link_elf_error(filename, "Unreadable program headers");

        ...

        /*    (multiplication of two Elf_Half fields will not overflow) */
        if ((hdr->e_phoff > PAGE_SIZE) ||
            (hdr->e_phentsize * hdr->e_phnum) > PAGE_SIZE - hdr->e_phoff) {
                error = ENOEXEC;
                goto fail;
        }

However, the same code to handle `e_shnum` and `e_shentsize` is reused multiple
times, and so multiple functions have the same bug, for example
`link_elf_load_file` in `sys/kern/link_elf.c`:

static int
link_elf_load_file(linker_class_t cls, const char* filename,
    linker_file_t* result)
{
        ...
        int nbytes;

        ...

        /*
         * Read the elf header from the file.
         */
        firstpage = malloc(PAGE_SIZE, M_LINKER, M_WAITOK);
        hdr = (Elf_Ehdr *)firstpage;
        error = vn_rdwr(UIO_READ, nd.ni_vp, firstpage, PAGE_SIZE, 0,
            UIO_SYSSPACE, IO_NODELOCKED, td->td_ucred, NOCRED,
            &resid, td);

        ...

        /*
         * Try and load the symbol table if it's present.  (you can
         * strip it!)
         */
        nbytes = hdr->e_shnum * hdr->e_shentsize;
        if (nbytes == 0 || hdr->e_shoff == 0)
                goto nosyms;
        shdr = malloc(nbytes, M_LINKER, M_WAITOK | Me_shnum_ZERO);
        error = vn_rdwr(UIO_READ, nd.ni_vp,
            (caddr_t)shdr, nbytes, hdr->e_shoff,
            UIO_SYSSPACE, IO_NODELOCKED, td->td_ucred, NOCRED,
            &resid, td);
        if (error != 0)
                goto out;
        symtabindex = -1;
        symstrindex = -1;
        for (i = 0; i < hdr->e_shnum; i++) {
                if (shdr[i].sh_type == SHT_SYMTAB) {
                        symtabindex = i;
                        symstrindex = shdr[i].sh_link;
                }
        }e_shnum

        ...
}

And `link_elf_load_file` from `sys/kern/link_elf_obj.c`.

-- 
You are receiving this mail because:
You are the assignee for the bug.
help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-206749-8>