[patch] prelink-20151030-gather_c-maybe_pie-oob-access.patch

prelink-20151030-gather_c-maybe_pie-oob-access.patch (text/plain), 6.05 KB, created by Alexander Miller on 2016-04-08 22:52:49 UTC

gather.c: fix out of bounds access in maybe_pie()

Patch submitted upstream.

The crashing function was created here:
  <https://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/commit/src/gather.c?id=efc75bd8a229fe99f60344150fbae3b71c7911de>
The out-of-bounds read was first introduced here, but it would ignore
program headers at a large offset, so no crashes commonly:
  <https://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/commit/trunk/src/gather.c?id=d0b5f973a7fc0136f5acbabcd20eb7161ddb147f>

--- prelink-cross-20151030/src/gather.c
+++ prelink-cross-20151030/src/gather.c
@@ -645,59 +645,89 @@ add_dir_to_dirlist (const char *name, de
   return 0;
 }
 
-/* Determine if a buffer holding an ELF header and program header
-   table may be that of a position-independent executable.  */
+/* Determine for a buffer holding an ELF header if the program header
+   table in the buffer or in the associated file may be that of a
+   position-independent executable.  */
 static int
-maybe_pie (unsigned char *e_ident, int big_endian, int sixty_four)
+maybe_pie (unsigned char *e_ident, size_t data_length, int fd,
+	   int big_endian, int sixty_four)
 {
   uint16_t num_phdrs;
   uint16_t phdr;
   size_t p_type_offset;
   size_t phnum_offset;
+  off_t phdr_offset;
+  size_t phentsize_offset;
+  uint16_t phentsize;
+  size_t phdr_size;
   unsigned char *phdr_table;
-  unsigned char *this_phdr;
+  size_t this_offset;
+  unsigned char buffer[0x1000];
 
   if (sixty_four)
     {
-      uint64_t phdr_offset;
-  
+      unsigned char *phoff = e_ident + offsetof (Elf64_Ehdr, e_phoff);
+      phdr_size = sizeof (Elf64_Phdr);
       p_type_offset = offsetof (Elf64_Phdr, p_type);
       phnum_offset = offsetof (Elf64_Ehdr, e_phnum);
+      phentsize_offset = offsetof (Elf64_Ehdr, e_phentsize);
       if (big_endian)
-        phdr_offset = buf_read_ube64 (&e_ident [offsetof (Elf64_Ehdr,
-                                                         e_phoff)]);
+        phdr_offset = buf_read_ube64 (phoff);
       else
-        phdr_offset = buf_read_ule64 (&e_ident [offsetof (Elf64_Ehdr,
-                                                         e_phoff)]);
-      phdr_table = e_ident + phdr_offset;
+        phdr_offset = buf_read_ule64 (phoff);
+      if (phdr_offset < sizeof (Elf64_Ehdr))
+	return 0;
     }
   else
     {
-      uint32_t phdr_offset;
-  
+      unsigned char *phoff = e_ident + offsetof (Elf32_Ehdr, e_phoff);
+      phdr_size = sizeof (Elf32_Phdr);
       p_type_offset = offsetof (Elf32_Phdr, p_type);
       phnum_offset = offsetof (Elf32_Ehdr, e_phnum);
+      phentsize_offset = offsetof (Elf32_Ehdr, e_phentsize);
       if (big_endian)
-        phdr_offset = buf_read_ube32 (&e_ident [offsetof (Elf32_Ehdr,
-                                                         e_phoff)]);
+        phdr_offset = buf_read_ube32 (phoff);
       else
-        phdr_offset = buf_read_ule32 (&e_ident [offsetof (Elf32_Ehdr,
-                                                         e_phoff)]);
-      phdr_table = e_ident + phdr_offset;
+        phdr_offset = buf_read_ule32 (phoff);
+      if (phdr_offset < sizeof (Elf32_Ehdr))
+	return 0;
     }
-
-  this_phdr = phdr_table;
 
   if (big_endian)
-    num_phdrs = buf_read_ube16 (&e_ident [phnum_offset]);
+    {
+      num_phdrs = buf_read_ube16 (e_ident + phnum_offset);
+      phentsize = buf_read_ube16 (e_ident + phentsize_offset);
+    }
   else
-    num_phdrs = buf_read_ule16 (&e_ident [phnum_offset]);
-
-  for (phdr = 0; phdr < num_phdrs; phdr++)
     {
-      unsigned char *p_type_start = this_phdr + p_type_offset;
+      num_phdrs = buf_read_ule16 (e_ident + phnum_offset);
+      phentsize = buf_read_ule16 (e_ident + phentsize_offset);
+    }
+  if (num_phdrs == 0 || phentsize < phdr_size)
+    return 0;
+  /* TODO: check that phdr_offset + phentsize * num_phdrs doesn't overflow */
+
+  phdr_table = e_ident;
+  this_offset = phdr_offset < data_length ? phdr_offset : data_length;
+  for (phdr = 0; phdr < num_phdrs; phdr++, this_offset += phentsize)
+    {
+      unsigned char *p_type_start;
       uint32_t p_type;
-  
+
+      if (this_offset + p_type_offset + sizeof (int32_t) > data_length)
+	{
+	  /* Read more headers from file */
+	  ssize_t read_bytes = pread (fd, buffer, sizeof(buffer),
+				      phdr_offset + phdr * (off_t) phentsize);
+	  if (read_bytes < phentsize)
+	    return 0;
+
+	  data_length = read_bytes;
+	  phdr_table = buffer;
+	  this_offset = 0;
+	}
+
+      p_type_start = phdr_table + this_offset + p_type_offset;
       if (big_endian)
        p_type = buf_read_ube32 (p_type_start);
       else
@@ -709,8 +739,6 @@ maybe_pie (unsigned char *e_ident, int b
       /* Any PT_PHDR entry must come before any PT_LOAD entry.  */
       if (p_type == PT_LOAD)
         return 0;
-  
-      this_phdr += sixty_four ? sizeof (Elf64_Phdr) : sizeof (Elf32_Phdr);
     }
     
   return 0;
@@ -720,7 +748,7 @@ static int
 gather_func (const char *name, const struct stat64 *st, int type,
 	     struct FTW *ftwp)
 {
-  unsigned char e_ident [sizeof (Elf64_Ehdr) + sizeof (Elf64_Phdr)];
+  unsigned char e_ident [sizeof (Elf64_Ehdr) + sizeof (Elf64_Phdr) + 4];
 
 #ifndef HAVE_FTW_ACTIONRETVAL
   if (blacklist_dir)
@@ -817,7 +845,7 @@ make_unprelinkable:
 		goto make_unprelinkable;
 	      else if (e_ident [EI_CLASS] == ELFCLASS32)
 		{
-		  if (maybe_pie (e_ident, 0, 0))
+		  if (maybe_pie (e_ident, sizeof (e_ident), fd, 0, 0))
 		    {
 maybe_pie:
 		      dso = fdopen_dso (fd, name);
@@ -834,7 +862,7 @@ maybe_pie:
 		}
 	      else if (e_ident [EI_CLASS] == ELFCLASS64)
 		{
-		  if (maybe_pie (e_ident, 0, 1))
+		  if (maybe_pie (e_ident, sizeof (e_ident), fd, 0, 1))
 		    goto maybe_pie;
 		  goto close_it;
 		}
@@ -851,13 +879,13 @@ maybe_pie:
 		goto make_unprelinkable;
 	      else if (e_ident [EI_CLASS] == ELFCLASS32)
 		{
-		  if (maybe_pie (e_ident, 1, 0))
+		  if (maybe_pie (e_ident, sizeof (e_ident), fd, 1, 0))
 		    goto maybe_pie;
 		  goto close_it;
 		}
 	      else if (e_ident [EI_CLASS] == ELFCLASS64)
 		{
-		  if (maybe_pie (e_ident, 1, 1))
+		  if (maybe_pie (e_ident, sizeof (e_ident), fd, 1, 1))
 		    goto maybe_pie;
 		  goto close_it;
 		}