Re: [patch] nlist(3): out of bounds read
Serguey Parkhomovsky wrote: > Ping? This is the same sanity check that's done in nm(1)'s ELF handling. Make sense to me. Tentative ok mmcc@ Alternatively, this check could be added to __elf_is_ok__, which is called right above where you added it. However, the definition of the function would have to change slightly; it's documented as checking whether the ELF header matches the target platform. > On Thu, Dec 10, 2015 at 09:40:11AM -0800, Serguey Parkhomovsky wrote: > > When dealing with a malformed ELF header, e_shentsize may be 0. This > > causes an out of bounds read while finding the symbol table on line 141. > > > > Found using afl. > > > > Index: nlist.c > > === > > RCS file: /cvs/src/lib/libc/gen/nlist.c,v > > retrieving revision 1.65 > > diff -u -p -r1.65 nlist.c > > --- nlist.c 16 Oct 2015 16:54:38 - 1.65 > > +++ nlist.c 10 Dec 2015 16:36:26 - > > @@ -102,6 +102,10 @@ __fdnlist(int fd, struct nlist *list) > > !__elf_is_okay__() || fstat(fd, ) < 0) > > return (-1); > > > > + /* Make sure section header size is not too small */ > > + if (ehdr.e_shentsize < sizeof(Elf_Shdr)) > > + return (-1); > > + > > /* calculate section header table size */ > > shdr_size = ehdr.e_shentsize * ehdr.e_shnum; > > >
Re: [patch] nlist(3): out of bounds read
Michael McConville wrote: > Serguey Parkhomovsky wrote: > > Ping? This is the same sanity check that's done in nm(1)'s ELF handling. > > Make sense to me. Tentative ok mmcc@ > > Alternatively, this check could be added to __elf_is_ok__, which is > called right above where you added it. However, the definition of the > function would have to change slightly; it's documented as checking > whether the ELF header matches the target platform. Here's a patch for that. I used the cleanest manner of patching in the check. __elf_is_ok__'s logic is pretty convoluted for a function that just returns the result of &&-ing a bunch of boolean conditions. We could turn the entire thing into a single return statement if we were so inclined. Index: lib/libc/gen/nlist.c === RCS file: /cvs/src/lib/libc/gen/nlist.c,v retrieving revision 1.65 diff -u -p -r1.65 nlist.c --- lib/libc/gen/nlist.c16 Oct 2015 16:54:38 - 1.65 +++ lib/libc/gen/nlist.c29 Dec 2015 05:08:09 - @@ -77,6 +77,9 @@ __elf_is_okay__(Elf_Ehdr *ehdr) retval = 1; } + if (ehdr->e_shentsize < sizeof(Elf_Shdr)) + return (0); + return retval; }
Re: [patch] nlist(3): out of bounds read
Ping? This is the same sanity check that's done in nm(1)'s ELF handling. On Thu, Dec 10, 2015 at 09:40:11AM -0800, Serguey Parkhomovsky wrote: > When dealing with a malformed ELF header, e_shentsize may be 0. This > causes an out of bounds read while finding the symbol table on line 141. > > Found using afl. > > Index: nlist.c > === > RCS file: /cvs/src/lib/libc/gen/nlist.c,v > retrieving revision 1.65 > diff -u -p -r1.65 nlist.c > --- nlist.c 16 Oct 2015 16:54:38 - 1.65 > +++ nlist.c 10 Dec 2015 16:36:26 - > @@ -102,6 +102,10 @@ __fdnlist(int fd, struct nlist *list) > !__elf_is_okay__() || fstat(fd, ) < 0) > return (-1); > > + /* Make sure section header size is not too small */ > + if (ehdr.e_shentsize < sizeof(Elf_Shdr)) > + return (-1); > + > /* calculate section header table size */ > shdr_size = ehdr.e_shentsize * ehdr.e_shnum; >
[patch] nlist(3): out of bounds read
When dealing with a malformed ELF header, e_shentsize may be 0. This causes an out of bounds read while finding the symbol table on line 141. Found using afl. Index: nlist.c === RCS file: /cvs/src/lib/libc/gen/nlist.c,v retrieving revision 1.65 diff -u -p -r1.65 nlist.c --- nlist.c 16 Oct 2015 16:54:38 - 1.65 +++ nlist.c 10 Dec 2015 16:36:26 - @@ -102,6 +102,10 @@ __fdnlist(int fd, struct nlist *list) !__elf_is_okay__() || fstat(fd, ) < 0) return (-1); + /* Make sure section header size is not too small */ + if (ehdr.e_shentsize < sizeof(Elf_Shdr)) + return (-1); + /* calculate section header table size */ shdr_size = ehdr.e_shentsize * ehdr.e_shnum;
