[v2] sprof: check pread size and offset for overflow

[DJ Delorie]

    Add a bit of descriptive paranoia to the values we read from
    the ELF headers and use to access data.

---- 8< ----

v2: rewrote check math

Paul Eggert <eggert@cs.ucla.edu> writes:
> On 2025-10-15 19:22, DJ Delorie wrote:
>> I want to do more than just check the sum, I want to check that every
>> aspect of this math is "inside" the file.  Any bad data is an error.
>> ...
>> #define PCHECK(s,l) if ((s) < 0  || (s) > st.st_size			\
>> 			|| (l) < 0 || (l) > st.st_size			\
>> 			|| ((s)+(l)) < 0 || ((s)+(l)) > st.st_size)	\
>
> The problem is that the last line does not do what you want. If S+L 
> overflows, behavior is undefined. This is why Collin suggested using 
> __builtin_add_overflow (or C23 ckd_add).

I rewrote the code to more closely check the underlying concerns, and
avoided the UB (I think).

diff --git a/elf/sprof.c b/elf/sprof.c
index c82c7c9db6..638281ffca 100644
--- a/elf/sprof.c
+++ b/elf/sprof.c
@@ -410,6 +410,7 @@ load_shobj (const char *name)
   int fd;
   ElfW(Shdr) *shdr;
   size_t pagesize = getpagesize ();
+  struct stat st;
 
   /* Since we use dlopen() we must be prepared to work around the sometimes
      strange lookup rules for the shared objects.  If we have a file foo.so
@@ -550,14 +551,34 @@ load_shobj (const char *name)
     error (EXIT_FAILURE, errno, _("Reopening shared object `%s' failed"),
 	   map->l_name);
 
+  if (fstat (fd, &st) < 0)
+    error (EXIT_FAILURE, errno, _("stat(%s) failure"), map->l_name);
+
+  /* We're depending on data that's being read from the file, so be a
+     bit paranoid here and make sure the requests are reasonable -
+     i.e. both size and offset are nonnegative and smaller than the
+     file size, as well as the offset of the end of the data.  PREAD
+     would have failed anyway, but this is more robust and explains
+     what happened better.  Note that SZ must be unsigned and OFF may
+     be signed or unsigned.  */
+#define PCHECK(sz,off) if ((sz) > st.st_size				\
+			   || (off_t)(off) < 0 || (off_t)(off) > st.st_size \
+			   ((sz)+(off_t)(off)) > st.st_size)		\
+    error (EXIT_FAILURE, ERANGE,					\
+	   _("read outside of file extents %zu + %zd > %zu"),		\
+	   (size_t)(sz), (off_t)(l), st.st_size)
+
   /* Map the section header.  */
   size_t size = ehdr->e_shnum * sizeof (ElfW(Shdr));
   shdr = (ElfW(Shdr) *) alloca (size);
+  PCHECK (size, ehdr->e_shoff);
   if (pread (fd, shdr, size, ehdr->e_shoff) != size)
     error (EXIT_FAILURE, errno, _("reading of section headers failed"));
 
   /* Get the section header string table.  */
   char *shstrtab = (char *) alloca (shdr[ehdr->e_shstrndx].sh_size);
+  PCHECK (shdr[ehdr->e_shstrndx].sh_size,
+	  shdr[ehdr->e_shstrndx].sh_offset);
   if (pread (fd, shstrtab, shdr[ehdr->e_shstrndx].sh_size,
 	     shdr[ehdr->e_shstrndx].sh_offset)
       != shdr[ehdr->e_shstrndx].sh_size)
@@ -585,6 +606,7 @@ load_shobj (const char *name)
       size_t size = debuglink_entry->sh_size;
       char *debuginfo_fname = (char *) alloca (size + 1);
       debuginfo_fname[size] = '\0';
+      PCHECK (size, debuglink_entry->sh_offset);
       if (pread (fd, debuginfo_fname, size, debuglink_entry->sh_offset)
 	  != size)
 	{
@@ -638,8 +660,13 @@ load_shobj (const char *name)
       if (fd2 != -1)
 	{
 	  ElfW(Ehdr) ehdr2;
+	  struct stat st;
+
+	  if (fstat (fd2, &st) < 0)
+	    error (EXIT_FAILURE, errno, _("stat(%s) failure"), workbuf);
 
 	  /* Read the ELF header.  */
+	  PCHECK (sizeof (ehdr2), 0);
 	  if (pread (fd2, &ehdr2, sizeof (ehdr2), 0) != sizeof (ehdr2))
 	    error (EXIT_FAILURE, errno,
 		   _("reading of ELF header failed"));
@@ -647,12 +674,15 @@ load_shobj (const char *name)
 	  /* Map the section header.  */
 	  size_t size = ehdr2.e_shnum * sizeof (ElfW(Shdr));
 	  ElfW(Shdr) *shdr2 = (ElfW(Shdr) *) alloca (size);
+	  PCHECK (size, ehdr2.e_shoff);
 	  if (pread (fd2, shdr2, size, ehdr2.e_shoff) != size)
 	    error (EXIT_FAILURE, errno,
 		   _("reading of section headers failed"));
 
 	  /* Get the section header string table.  */
 	  shstrtab = (char *) alloca (shdr2[ehdr2.e_shstrndx].sh_size);
+	  PCHECK (shdr2[ehdr2.e_shstrndx].sh_size,
+		  shdr2[ehdr2.e_shstrndx].sh_offset);
 	  if (pread (fd2, shstrtab, shdr2[ehdr2.e_shstrndx].sh_size,
 		     shdr2[ehdr2.e_shstrndx].sh_offset)
 	      != shdr2[ehdr2.e_shstrndx].sh_size)