From e239ecc1fd518311e4a4fd95ecaeb245f5ed9230 Mon Sep 17 00:00:00 2001
From: tobias <tobias@openbsd.org>
Date: Fri, 16 Oct 2015 07:40:12 +0000
Subject: [PATCH] Validate parsed ELF values to prevent out of boundary
 accesses. While at it, return proper return value when encountering a
 stripped binary. Instead of -1 (illegal file), it should be the amount of
 symbols that were tried to be resolved.

ok millert
---
 lib/libc/gen/nlist.c | 77 +++++++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 33 deletions(-)

diff --git a/lib/libc/gen/nlist.c b/lib/libc/gen/nlist.c
index 100fa942a88..f9666265c66 100644
--- a/lib/libc/gen/nlist.c
+++ b/lib/libc/gen/nlist.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nlist.c,v 1.62 2015/10/04 06:59:57 guenther Exp $ */
+/*	$OpenBSD: nlist.c,v 1.63 2015/10/16 07:40:12 tobias Exp $ */
 /*
  * Copyright (c) 1989, 1993
  *	The Regents of the University of California.  All rights reserved.
@@ -95,6 +95,7 @@ __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) ||
@@ -105,7 +106,8 @@ __fdnlist(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_MAX) {
+	if (SIZE_MAX - ehdr.e_shoff < shdr_size ||
+	    ehdr.e_shoff + shdr_size > st.st_size) {
 		errno = EFBIG;
 		return (-1);
 	}
@@ -132,6 +134,8 @@ __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;
@@ -146,12 +150,37 @@ __fdnlist(int fd, struct nlist *list)
 	else
 		munmap((caddr_t)shdr, shdr_size);
 
+	/*
+	 * clean out any left-over information for all valid entries.
+	 * Type and value defined to be 0 if not found; historical
+	 * versions cleared other and desc as well.  Also figure out
+	 * the largest string length so don't read any more of the
+	 * string table than we have to.
+	 *
+	 * XXX clearing anything other than n_type and n_value violates
+	 * the semantics given in the man page.
+	 */
+	nent = 0;
+	for (p = list; !ISLAST(p); ++p) {
+		p->n_type = 0;
+		p->n_other = 0;
+		p->n_desc = 0;
+		p->n_value = 0;
+		++nent;
+	}
+
+	/* Don't process any further if object is stripped. */
+	/* ELFism - dunno if stripped by looking at header */
+	if (symoff == 0)
+		return nent;
+
 	/* Check for files too large to mmap. */
-	/* XXX is this really possible? */
-	if (symstrsize > SIZE_MAX) {
+	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
@@ -171,41 +200,20 @@ __fdnlist(int fd, struct nlist *list)
 		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
-	 * versions cleared other and desc as well.  Also figure out
-	 * the largest string length so don't read any more of the
-	 * string table than we have to.
-	 *
-	 * XXX clearing anything other than n_type and n_value violates
-	 * the semantics given in the man page.
-	 */
-	nent = 0;
-	for (p = list; !ISLAST(p); ++p) {
-		p->n_type = 0;
-		p->n_other = 0;
-		p->n_desc = 0;
-		p->n_value = 0;
-		++nent;
-	}
 
-	/* Don't process any further if object is stripped. */
-	/* ELFism - dunno if stripped by looking at header */
-	if (symoff == 0)
-		goto elf_done;
-
-	while (symsize > 0) {
+	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;
 
@@ -215,13 +223,16 @@ __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_un.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;
 
-- 
2.20.1