From: tobias Date: Fri, 16 Oct 2015 13:54:45 +0000 (+0000) Subject: Merge nlist out of boundary access fix with other nlist implementations. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4497e20808fcf8a7ec1b3b5c697ae9b85db6a4f8;p=openbsd Merge nlist out of boundary access fix with other nlist implementations. While at it, merge style and typo fixes back into nlist(3), too. ok deraadt, jsing, millert --- diff --git a/distrib/common/elfrd_size.c b/distrib/common/elfrd_size.c index a7817f03aa5..58458012f7e 100644 --- a/distrib/common/elfrd_size.c +++ b/distrib/common/elfrd_size.c @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -132,7 +133,7 @@ ELFNAME(find_rd_root_image)(char *file, int fd, Elf_Phdr *ph, int segment, * is ELF and valid for the target platform. * * WARNING: This is NOT a ELF ABI function and - * as such it's use should be restricted. + * as such its use should be restricted. */ int ELFNAME(__elf_is_okay__)(Elf_Ehdr *ehdr) @@ -149,7 +150,7 @@ ELFNAME(__elf_is_okay__)(Elf_Ehdr *ehdr) ehdr->e_ident[EI_VERSION] == ELF_TARG_VER) { #if 0 /* allow cross, no arch check */ - /* Now check the machine dependant header */ + /* Now check the machine dependent header */ if (ehdr->e_machine == ELF_TARG_MACH && ehdr->e_version == ELF_TARG_VER) #endif @@ -179,6 +180,7 @@ ELFNAME(nlist)(int fd, struct nlist *list) size_t shdr_size; struct stat st; int usemalloc = 0; + size_t left, len; /* Make sure obj is OK */ if (pread(fd, &ehdr, sizeof(Elf_Ehdr), (off_t)0) != sizeof(Elf_Ehdr) || @@ -189,7 +191,8 @@ ELFNAME(nlist)(int fd, struct nlist *list) shdr_size = ehdr.e_shentsize * ehdr.e_shnum; /* Make sure it's not too big to mmap */ - if (shdr_size > SIZE_T_MAX) { + if (SIZE_MAX - ehdr.e_shoff < shdr_size || + ehdr.e_shoff + shdr_size > st.st_size) { errno = EFBIG; return (-1); } @@ -202,20 +205,23 @@ ELFNAME(nlist)(int fd, struct nlist *list) if ((shdr = malloc(shdr_size)) == NULL) return (-1); - if (pread(fd, shdr, shdr_size, (off_t)ehdr.e_shoff) != shdr_size) { + if (pread(fd, shdr, shdr_size, (off_t)ehdr.e_shoff) != + shdr_size) { free(shdr); return (-1); } } /* - * Find the symbol table entry and it's corresponding + * Find the symbol table entry and its corresponding * string table entry. Version 1.1 of the ABI states * that there is only one symbol table but that this * could change in the future. */ for (i = 0; i < ehdr.e_shnum; i++) { if (shdr[i].sh_type == SHT_SYMTAB) { + if (shdr[i].sh_link >= ehdr.e_shnum) + continue; symoff = shdr[i].sh_offset; symsize = shdr[i].sh_size; symstroff = shdr[shdr[i].sh_link].sh_offset; @@ -230,31 +236,6 @@ ELFNAME(nlist)(int fd, struct nlist *list) else munmap((caddr_t)shdr, shdr_size); - /* Check for files too large to mmap. */ - /* XXX is this really possible? */ - if (symstrsize > SIZE_T_MAX) { - errno = EFBIG; - return (-1); - } - /* - * Map string table into our address space. This gives us - * an easy way to randomly access all the strings, without - * making the memory allocation permanent as with malloc/free - * (i.e., munmap will return it to the system). - */ - if (usemalloc) { - if ((strtab = malloc(symstrsize)) == NULL) - return (-1); - if (pread(fd, strtab, symstrsize, (off_t)symstroff) != symstrsize) { - free(strtab); - return (-1); - } - } else { - strtab = mmap(NULL, (size_t)symstrsize, PROT_READ, - MAP_SHARED|MAP_FILE, fd, (off_t) symstroff); - if (strtab == MAP_FAILED) - return (-1); - } /* * clean out any left-over information for all valid entries. * Type and value defined to be 0 if not found; historical @@ -277,19 +258,49 @@ ELFNAME(nlist)(int fd, struct nlist *list) /* Don't process any further if object is stripped. */ /* ELFism - dunno if stripped by looking at header */ if (symoff == 0) - goto elf_done; + return nent; - while (symsize > 0) { + /* Check for files too large to mmap. */ + if (SIZE_MAX - symstrsize < symstroff || + symstrsize + symstroff > st.st_size) { + errno = EFBIG; + return (-1); + } + + /* + * Map string table into our address space. This gives us + * an easy way to randomly access all the strings, without + * making the memory allocation permanent as with malloc/free + * (i.e., munmap will return it to the system). + */ + if (usemalloc) { + if ((strtab = malloc(symstrsize)) == NULL) + return (-1); + if (pread(fd, strtab, symstrsize, (off_t)symstroff) != + symstrsize) { + free(strtab); + return (-1); + } + } else { + strtab = mmap(NULL, (size_t)symstrsize, PROT_READ, + MAP_SHARED|MAP_FILE, fd, (off_t) symstroff); + if (strtab == MAP_FAILED) + return (-1); + } + + while (symsize >= sizeof(Elf_Sym)) { cc = MIN(symsize, sizeof(sbuf)); if (pread(fd, sbuf, cc, (off_t)symoff) != cc) break; symsize -= cc; symoff += cc; for (s = sbuf; cc > 0; ++s, cc -= sizeof(*s)) { - int soff = s->st_name; + Elf_Word soff = s->st_name; - if (soff == 0) + if (soff == 0 || soff >= symstrsize) continue; + left = symstrsize - soff; + for (p = list; !ISLAST(p); p++) { char *sym; @@ -299,13 +310,16 @@ ELFNAME(nlist)(int fd, struct nlist *list) * and the first char is an '_', skip over * the '_' and try again. * XXX - What do we do when the user really - * wants '_foo' and the are symbols + * wants '_foo' and there are symbols * for both 'foo' and '_foo' in the * table and 'foo' is first? */ sym = p->n_name; - if (strcmp(&strtab[soff], sym) != 0 && - (sym[0] != '_' || + len = strlen(sym); + + if ((len >= left || + strcmp(&strtab[soff], sym) != 0) && + (sym[0] != '_' || len - 1 >= left || strcmp(&strtab[soff], sym + 1) != 0)) continue; diff --git a/lib/libc/gen/nlist.c b/lib/libc/gen/nlist.c index f9666265c66..437640732d1 100644 --- a/lib/libc/gen/nlist.c +++ b/lib/libc/gen/nlist.c @@ -1,4 +1,4 @@ -/* $OpenBSD: nlist.c,v 1.63 2015/10/16 07:40:12 tobias Exp $ */ +/* $OpenBSD: nlist.c,v 1.64 2015/10/16 13:54:45 tobias Exp $ */ /* * Copyright (c) 1989, 1993 * The Regents of the University of California. All rights reserved. @@ -71,7 +71,7 @@ __elf_is_okay__(Elf_Ehdr *ehdr) ehdr->e_ident[EI_DATA] == ELF_TARG_DATA && ehdr->e_ident[EI_VERSION] == ELF_TARG_VER) { - /* Now check the machine dependant header */ + /* Now check the machine dependent header */ if (ehdr->e_machine == ELF_TARG_MACH && ehdr->e_version == ELF_TARG_VER) retval = 1; @@ -120,7 +120,8 @@ __fdnlist(int fd, struct nlist *list) if ((shdr = malloc(shdr_size)) == NULL) return (-1); - if (pread(fd, shdr, shdr_size, (off_t)ehdr.e_shoff) != shdr_size) { + if (pread(fd, shdr, shdr_size, (off_t)ehdr.e_shoff) != + shdr_size) { free(shdr); return (-1); } @@ -190,7 +191,8 @@ __fdnlist(int fd, struct nlist *list) if (usemalloc) { if ((strtab = malloc(symstrsize)) == NULL) return (-1); - if (pread(fd, strtab, symstrsize, (off_t)symstroff) != symstrsize) { + if (pread(fd, strtab, symstrsize, (off_t)symstroff) != + symstrsize) { free(strtab); return (-1); } @@ -267,8 +269,7 @@ __fdnlist(int fd, struct nlist *list) p->n_type = N_FN; break; } - if (ELF_ST_BIND(s->st_info) == - STB_LOCAL) + if (ELF_ST_BIND(s->st_info) == STB_LOCAL) p->n_type = N_EXT; p->n_desc = 0; p->n_other = 0; diff --git a/usr.sbin/installboot/i386_nlist.c b/usr.sbin/installboot/i386_nlist.c index 23d19cc87f6..9d5950dcbf7 100644 --- a/usr.sbin/installboot/i386_nlist.c +++ b/usr.sbin/installboot/i386_nlist.c @@ -1,4 +1,4 @@ -/* $OpenBSD: i386_nlist.c,v 1.2 2015/01/16 00:05:12 deraadt Exp $ */ +/* $OpenBSD: i386_nlist.c,v 1.3 2015/10/16 13:54:45 tobias Exp $ */ /* * Copyright (c) 1989, 1993 * The Regents of the University of California. All rights reserved. @@ -35,8 +35,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -79,7 +81,7 @@ __elf_is_okay__(Elf_Ehdr *ehdr) ehdr->e_ident[EI_DATA] == ELF_TARG_DATA && ehdr->e_ident[EI_VERSION] == ELF_TARG_VER) { - /* Now check the machine dependant header */ + /* Now check the machine dependent header */ if (ehdr->e_machine == EM_386 && ehdr->e_version == ELF_TARG_VER) retval = 1; @@ -103,6 +105,7 @@ __elf_fdnlist(int fd, struct nlist *list) Elf_Word shdr_size; struct stat st; int usemalloc = 0; + size_t left, len; /* Make sure obj is OK */ if (pread(fd, &ehdr, sizeof(Elf_Ehdr), (off_t)0) != sizeof(Elf_Ehdr) || @@ -112,6 +115,13 @@ __elf_fdnlist(int fd, struct nlist *list) /* calculate section header table size */ shdr_size = ehdr.e_shentsize * ehdr.e_shnum; + /* Make sure it's not too big to mmap */ + if (SIZE_MAX - ehdr.e_shoff < shdr_size || + ehdr.e_shoff + shdr_size > st.st_size) { + errno = EFBIG; + return (-1); + } + /* mmap section header table */ shdr = (Elf_Shdr *)mmap(NULL, (size_t)shdr_size, PROT_READ, MAP_SHARED|MAP_FILE, fd, (off_t) ehdr.e_shoff); @@ -135,6 +145,8 @@ __elf_fdnlist(int fd, struct nlist *list) */ for (i = 0; i < ehdr.e_shnum; i++) { if (shdr[i].sh_type == SHT_SYMTAB) { + if (shdr[i].sh_link >= ehdr.e_shnum) + continue; symoff = shdr[i].sh_offset; symsize = shdr[i].sh_size; symstroff = shdr[shdr[i].sh_link].sh_offset; @@ -149,26 +161,6 @@ __elf_fdnlist(int fd, struct nlist *list) else munmap((caddr_t)shdr, shdr_size); - /* - * Map string table into our address space. This gives us - * an easy way to randomly access all the strings, without - * making the memory allocation permanent as with malloc/free - * (i.e., munmap will return it to the system). - */ - if (usemalloc) { - if ((strtab = malloc(symstrsize)) == NULL) - return (-1); - if (pread(fd, strtab, symstrsize, (off_t)symstroff) != - symstrsize) { - free(strtab); - return (-1); - } - } else { - strtab = mmap(NULL, (size_t)symstrsize, PROT_READ, - MAP_SHARED|MAP_FILE, fd, (off_t) symstroff); - if (strtab == MAP_FAILED) - return (-1); - } /* * clean out any left-over information for all valid entries. * Type and value defined to be 0 if not found; historical @@ -191,19 +183,49 @@ __elf_fdnlist(int fd, struct nlist *list) /* Don't process any further if object is stripped. */ /* ELFism - dunno if stripped by looking at header */ if (symoff == 0) - goto elf_done; + return nent; + + /* Check for files too large to mmap. */ + if (SIZE_MAX - symstrsize < symstroff || + symstrsize + symstroff > st.st_size) { + errno = EFBIG; + return (-1); + } - while (symsize > 0) { + /* + * Map string table into our address space. This gives us + * an easy way to randomly access all the strings, without + * making the memory allocation permanent as with malloc/free + * (i.e., munmap will return it to the system). + */ + if (usemalloc) { + if ((strtab = malloc(symstrsize)) == NULL) + return (-1); + if (pread(fd, strtab, symstrsize, (off_t)symstroff) != + symstrsize) { + free(strtab); + return (-1); + } + } else { + strtab = mmap(NULL, (size_t)symstrsize, PROT_READ, + MAP_SHARED|MAP_FILE, fd, (off_t) symstroff); + if (strtab == MAP_FAILED) + return (-1); + } + + while (symsize >= sizeof(Elf_Sym)) { cc = MINIMUM(symsize, sizeof(sbuf)); if (pread(fd, sbuf, cc, (off_t)symoff) != cc) break; symsize -= cc; symoff += cc; for (s = sbuf; cc > 0; ++s, cc -= sizeof(*s)) { - int soff = s->st_name; + Elf_Word soff = s->st_name; - if (soff == 0) + if (soff == 0 || soff >= symstrsize) continue; + left = symstrsize - soff; + for (p = list; !ISLAST(p); p++) { char *sym; @@ -213,13 +235,16 @@ __elf_fdnlist(int fd, struct nlist *list) * and the first char is an '_', skip over * the '_' and try again. * XXX - What do we do when the user really - * wants '_foo' and the are symbols + * wants '_foo' and there are symbols * for both 'foo' and '_foo' in the * table and 'foo' is first? */ sym = p->n_name; - if (strcmp(&strtab[soff], sym) != 0 && - (sym[0] != '_' || + len = strlen(sym); + + if ((len >= left || + strcmp(&strtab[soff], sym) != 0) && + (sym[0] != '_' || len - 1 >= left || strcmp(&strtab[soff], sym + 1) != 0)) continue; @@ -254,8 +279,7 @@ __elf_fdnlist(int fd, struct nlist *list) p->n_type = N_FN; break; } - if (ELF_ST_BIND(s->st_info) == - STB_LOCAL) + if (ELF_ST_BIND(s->st_info) == STB_LOCAL) p->n_type = N_EXT; p->n_desc = 0; p->n_other = 0;