commit 0b535f4d8e08307122f08323fac93331b60cddf8
author Christopher Ferris <cferris@google.com> Thu May 14 18:53:59 2015 -0700
committer Christopher Ferris <cferris@google.com> Fri May 15 15:53:54 2015 -0700
tree 5ae6de8ce68accff013fc136bd20f8955d6f1088
parent 7124451dfc655b24e549ded8594c449393ab7597

Check for overflow when getting symbol name.

If there is a malformed elf with a particularly large st_name value,
then an overflow can occur. Check for this condition and ignore the
symbols that would cause an overflow.

Bug: 21162746
(cherry picked from commit 14de40d95372863fff4f460fb7e128404d87f8f4)

Change-Id: Icf44695b83b8371e176b9f303e43a182bd7bed4e

src/elfxx.c

diff --git a/src/elfxx.c b/src/elfxx.c
index 8f185a8..ec30239 100644
--- a/src/elfxx.c
+++ b/src/elfxx.c
@@ -208,24 +208,27 @@
             if (ip >= val && (Elf_W(Addr)) (ip - val) < sym.st_size) {
               GET_SYM_FIELD(ei, sym_offset, &sym, st_name);
               uintptr_t size = ei->u.memory.map->end - ei->u.memory.map->start;
-              if (strtab_offset + sym.st_name > size) {
+              Elf_W(Off) strname_offset = strtab_offset + sym.st_name;
+              if (strname_offset > size || strname_offset < strtab_offset) {
                 // Malformed elf symbol table.
                 break;
               }
 
               size_t bytes_read = elf_w (memory_read) (
-                  ei, ei->u.memory.map->start + strtab_offset + sym.st_name,
+                  ei, ei->u.memory.map->start + strname_offset,
                   (uint8_t*) buf, buf_len, true);
               if (bytes_read == 0) {
                 // Empty name, so keep checking the other symbol tables
                 // for a possible match.
                 break;
               }
+              // Ensure the string is nul terminated, it is assumed that
+              // sizeof(buf) >= buf_len + 1.
+              buf[buf_len] = '\0';
+
               if (offp != NULL) {
                 *offp = ip - val;
               }
-              // Ensure the string is nul terminated.
-              buf[buf_len] = '\0';
               return true;
             }
           }
@@ -343,19 +346,23 @@
             }
             Debug (16, "0x%016lx info=0x%02x\n", (long) val, sym->st_info);
             if (ip >= val && (Elf_W(Addr)) (ip - val) < sym->st_size) {
-              if (strtab + sym->st_name > (char*) ei->u.mapped.image + ei->u.mapped.size) {
+              char* str_name = strtab + sym->st_name;
+              if (str_name > (char*) ei->u.mapped.image + ei->u.mapped.size ||
+                  str_name < strtab) {
                 // Malformed elf symbol table.
                 break;
               }
 
               // Make sure we don't try and read past the end of the image.
-              uintptr_t max_size = (uintptr_t) (strtab + sym->st_name)
-                  - (uintptr_t) ei->u.mapped.image;
+              uintptr_t max_size = (uintptr_t) str_name - (uintptr_t) ei->u.mapped.image;
               if (buf_len > max_size) {
                 buf_len = max_size;
               }
 
-              strncpy (buf, strtab + sym->st_name, buf_len);
+              strncpy (buf, str_name, buf_len);
+              // Ensure the string is nul terminated, it is assumed that
+              // sizeof(buf) >= buf_len + 1.
+              buf[buf_len] = '\0';
               if (buf[0] == '\0') {
                 // Empty name, so keep checking the other symbol tables
                 // for a possible match.
@@ -364,8 +371,6 @@
               if (offp != NULL) {
                 *offp = ip - val;
               }
-              // Ensure the string is nul terminated.
-              buf[buf_len] = '\0';
               return true;
             }
           }