CVE-2022-38533: Out-of-bounds Write (CWE-787)
value: In GNU Binutils before 2.40, there is a heap-buffer-overflow in the error function bfd_getl32 when called from the strip_main function in strip-new via a crafted file.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2022-08-26T00:15Z
Repository: https://github.com/bminor/binutils-gdb
Language: C
Files changed: bfd/elf.c
Commit date: 2022-08-16 18:19:34 +0930
Commit message: PR29495, rewrite_elf_program_header looping

This patch, in order of significance:
1) Replaces some macros with inline functions.
2) Those inline functions catch and avoid arithmetic overflows when
   comparing addresses.
3) When assigning sections to segments (IS_SECTION_IN_INPUT_SEGMENT)
   use bed->want_p_paddr_set_to_zero to decide whether lma vs p_paddr
   or vma vs p_vaddr should be tested.  When remapping, use the same
   test, and use is_note rather than the more restrictive
   IS_COREFILE_NOTE.

It's important that the later tests not be more restrictive.  If they
are it can lead to the situation triggered by the testcases, where a
section seemingly didn't fit and thus needed a new mapping.  It didn't
fit the new mapping either, and this repeated until memory exhausted.

	PR 29495
	* elf.c (SEGMENT_END, SECTION_SIZE, IS_CONTAINED_BY_VMA): Delete.
	(IS_CONTAINED_BY_LMA, IS_NOTE, IS_COREFILE_NOTE): Delete.
	(segment_size, segment_end, section_size): New inline function.
	(is_contained_by, is_note): Likewise.
	(rewrite_elf_program_header): Use new functions.

Diff: diff --git a/bfd/elf.c b/bfd/elf.c
index 1b9b7fad15..76877b43f9 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -6884,10 +6884,70 @@ _bfd_elf_symbol_from_bfd_symbol (bfd *abfd, asymbol **asym_ptr_ptr)
 #endif
 
   return idx;
 }
 
+static inline bfd_vma
+segment_size (Elf_Internal_Phdr *segment)
+{
+  return (segment->p_memsz > segment->p_filesz
+	  ? segment->p_memsz : segment->p_filesz);
+}
+
+
+/* Returns the end address of the segment + 1.  */
+static inline bfd_vma
+segment_end (Elf_Internal_Phdr *segment, bfd_vma start)
+{
+  return start + segment_size (segment);
+}
+
+static inline bfd_size_type
+section_size (asection *section, Elf_Internal_Phdr *segment)
+{
+  if ((section->flags & SEC_HAS_CONTENTS) != 0
+      || (section->flags & SEC_THREAD_LOCAL) == 0
+      || segment->p_type == PT_TLS)
+    return section->size;
+  return 0;
+}
+
+/* Returns TRUE if the given section is contained within the given
+   segment.  LMA addresses are compared against PADDR when
+   bed->want_p_paddr_set_to_zero is false, VMA against VADDR when true.  */
+static bool
+is_contained_by (asection *section, Elf_Internal_Phdr *segment,
+		 bfd_vma paddr, bfd_vma vaddr, unsigned int opb,
+		 const struct elf_backend_data *bed)
+{
+  bfd_vma seg_addr = !bed->want_p_paddr_set_to_zero ? paddr : vaddr;
+  bfd_vma addr = !bed->want_p_paddr_set_to_zero ? section->lma : section->vma;
+  bfd_vma octet;
+  if (_bfd_mul_overflow (addr, opb, &octet))
+    return false;
+  /* The third and fourth lines below are testing that the section end
+     address is within the segment.  It's written this way to avoid
+     overflow.  Add seg_addr + section_size to both sides of the
+     inequality to make it obvious.  */
+  return (octet >= seg_addr
+	  && segment_size (segment) >= section_size (section, segment)
+	  && (octet - seg_addr
+	      <= segment_size (segment) - section_size (section, segment)));
+}
+
+/* Handle PT_NOTE segment.  */
+static bool
+is_note (Elf_Internal_Phdr *p, asection *s)
+{
+  return (p->p_type == PT_NOTE
+	  && elf_section_type (s) == SHT_NOTE
+	  && (ufile_ptr) s->filepos >= p->p_offset
+	  && p->p_filesz >= s->size
+	  && ((ufile_ptr) s->filepos - p->p_offset
+	      <= p->p_filesz - s->size));
+}
+
 /* Rewrite program header information.  */
 
 static bool
 rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
 {
@@ -6912,51 +6972,10 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
   map_first = NULL;
   pointer_to_map = &map_first;
 
   num_segments = elf_elfheader (ibfd)->e_phnum;
 
-  /* Returns the end address of the segment + 1.  */
-#define SEGMENT_END(segment, start)					\
-  (start + (segment->p_memsz > segment->p_filesz			\
-	    ? segment->p_memsz : segment->p_filesz))
-
-#define SECTION_SIZE(section, segment)					\
-  (((section->flags & (SEC_HAS_CONTENTS | SEC_THREAD_LOCAL))		\
-    != SEC_THREAD_LOCAL || segment->p_type == PT_TLS)			\
-   ? section->size : 0)
-
-  /* Returns TRUE if the given section is contained within
-     the given segment.  VMA addresses are compared.  */
-#define IS_CONTAINED_BY_VMA(section, segment, opb)			\
-  (section->vma * (opb) >= segment->p_vaddr				\
-   && (section->vma * (opb) + SECTION_SIZE (section, segment)		\
-       <= (SEGMENT_END (segment, segment->p_vaddr))))
-
-  /* Returns TRUE if the given section is contained within
-     the given segment.  LMA addresses are compared.  */
-#define IS_CONTAINED_BY_LMA(section, segment, base, opb)		\
-  (section->lma * (opb) >= base						\
-   && (section->lma + SECTION_SIZE (section, segment) / (opb) >= section->lma) \
-   && (section->lma * (opb) + SECTION_SIZE (section, segment)		\
-       <= SEGMENT_END (segment, base)))
-
-  /* Handle PT_NOTE segment.  */
-#define IS_NOTE(p, s)							\
-  (p->p_type == PT_NOTE							\
-   && elf_section_type (s) == SHT_NOTE					\
-   && (bfd_vma) s->filepos >= p->p_offset				\
-   && ((bfd_vma) s->filepos + s->size					\
-       <= p->p_offset + p->p_filesz))
-
-  /* Special case: corefile "NOTE" section containing regs, prpsinfo
-     etc.  */
-#define IS_COREFILE_NOTE(p, s)						\
-  (IS_NOTE (p, s)							\
-   && bfd_get_format (ibfd) == bfd_core					\
-   && s->vma == 0							\
-   && s->lma == 0)
-
   /* The complicated case when p_vaddr is 0 is to handle the Solaris
      linker, which generates a PT_INTERP section with p_vaddr and
      p_memsz set to 0.  */
 #define IS_SOLARIS_PT_INTERP(p, s)					\
   (p->p_vaddr == 0							\
@@ -6981,23 +7000,22 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
        6. PT_TLS segment includes only SHF_TLS sections.
        7. SHF_TLS sections are only in PT_TLS or PT_LOAD segments.
        8. PT_DYNAMIC should not contain empty sections at the beginning
 	  (with the possible exception of .dynamic).  */
 #define IS_SECTION_IN_INPUT_SEGMENT(section, segment, bed, opb)		\
-  ((((segment->p_paddr							\
-      ? IS_CONTAINED_BY_LMA (section, segment, segment->p_paddr, opb)	\
-      : IS_CONTAINED_BY_VMA (section, segment, opb))			\
+  (((is_contained_by (section, segment, segment->p_paddr,		\
+		      segment->p_vaddr, opb, bed)			\
      && (section->flags & SEC_ALLOC) != 0)				\
-    || IS_NOTE (segment, section))					\
+    || is_note (segment, section))					\
    && segment->p_type != PT_GNU_STACK					\
    && (segment->p_type != PT_TLS					\
        || (section->flags & SEC_THREAD_LOCAL))				\
    && (segment->p_type == PT_LOAD					\
        || segment->p_type == PT_TLS					\
        || (section->flags & SEC_THREAD_LOCAL) == 0)			\
    && (segment->p_type != PT_DYNAMIC					\
-       || SECTION_SIZE (section, segment) > 0				\
+       || section_size (section, segment) > 0				\
        || (segment->p_paddr						\
 	   ? segment->p_paddr != section->lma * (opb)			\
 	   : segment->p_vaddr != section->vma * (opb))			\
        || (strcmp (bfd_section_name (section), ".dynamic") == 0))	\
    && (segment->p_type != PT_LOAD || !section->segment_mark))
@@ -7008,11 +7026,11 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
   (IS_SECTION_IN_INPUT_SEGMENT (section, segment, bed, opb)		\
    && section->output_section != NULL)
 
   /* Returns TRUE iff seg1 starts after the end of seg2.  */
 #define SEGMENT_AFTER_SEGMENT(seg1, seg2, field)			\
-  (seg1->field >= SEGMENT_END (seg2, seg2->field))
+  (seg1->field >= segment_end (seg2, seg2->field))
 
   /* Returns TRUE iff seg1 and seg2 overlap. Segments overlap iff both
      their VMA address ranges and their LMA address ranges overlap.
      It is possible to have overlapping VMA ranges without overlapping LMA
      ranges.  RedBoot images for example can have both .data and .bss mapped
@@ -7088,12 +7106,12 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
 	  /* Merge the two segments together.  */
 	  if (segment2->p_vaddr < segment->p_vaddr)
 	    {
 	      /* Extend SEGMENT2 to include SEGMENT and then delete
 		 SEGMENT.  */
-	      extra_length = (SEGMENT_END (segment, segment->p_vaddr)
-			      - SEGMENT_END (segment2, segment2->p_vaddr));
+	      extra_length = (segment_end (segment, segment->p_vaddr)
+			      - segment_end (segment2, segment2->p_vaddr));
 
 	      if (extra_length > 0)
 		{
 		  segment2->p_memsz += extra_length;
 		  segment2->p_filesz += extra_length;
@@ -7108,12 +7126,12 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
 	    }
 	  else
 	    {
 	      /* Extend SEGMENT to include SEGMENT2 and then delete
 		 SEGMENT2.  */
-	      extra_length = (SEGMENT_END (segment2, segment2->p_vaddr)
-			      - SEGMENT_END (segment, segment->p_vaddr));
+	      extra_length = (segment_end (segment2, segment2->p_vaddr)
+			      - segment_end (segment, segment->p_vaddr));
 
 	      if (extra_length > 0)
 		{
 		  segment->p_memsz += extra_length;
 		  segment->p_filesz += extra_length;
@@ -7309,15 +7327,13 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
 		      == (output_section->vma * opb)))
 		map->p_paddr = segment->p_vaddr;
 
 	      /* Match up the physical address of the segment with the
 		 LMA address of the output section.  */
-	      if (IS_CONTAINED_BY_LMA (output_section, segment, map->p_paddr,
-				       opb)
-		  || IS_COREFILE_NOTE (segment, section)
-		  || (bed->want_p_paddr_set_to_zero
-		      && IS_CONTAINED_BY_VMA (output_section, segment, opb)))
+	      if (is_contained_by (output_section, segment, map->p_paddr,
+				   map->p_paddr + map->p_vaddr_offset, opb, bed)
+		  || is_note (segment, section))
 		{
 		  if (matching_lma == NULL
 		      || output_section->lma < matching_lma->lma)
 		    matching_lma = output_section;
 
@@ -7429,13 +7445,13 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
 
 	      output_section = section->output_section;
 
 	      BFD_ASSERT (output_section != NULL);
 
-	      if (IS_CONTAINED_BY_LMA (output_section, segment, map->p_paddr,
-				       opb)
-		  || IS_COREFILE_NOTE (segment, section))
+	      if (is_contained_by (output_section, segment, map->p_paddr,
+				   map->p_paddr + map->p_vaddr_offset, opb, bed)
+		  || is_note (segment, section))
 		{
 		  if (map->count == 0)
 		    {
 		      /* If the first section in a segment does not start at
 			 the beginning of the segment, then something is
@@ -7554,16 +7570,10 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize)
 	    map->p_paddr = phdr_adjust_seg->p_paddr + adjust;
 	    break;
 	  }
     }
 
-#undef SEGMENT_END
-#undef SECTION_SIZE
-#undef IS_CONTAINED_BY_VMA
-#undef IS_CONTAINED_BY_LMA
-#undef IS_NOTE
-#undef IS_COREFILE_NOTE
 #undef IS_SOLARIS_PT_INTERP
 #undef IS_SECTION_IN_INPUT_SEGMENT
 #undef INCLUDE_SECTION_IN_SEGMENT
 #undef SEGMENT_AFTER_SEGMENT
 #undef SEGMENT_OVERLAPS

Vulnerable code:   /* Returns the end address of the segment + 1.  */
#define SEGMENT_END(segment, start)					\
  (start + (segment->p_memsz > segment->p_filesz			\
	    ? segment->p_memsz : segment->p_filesz))

#define SECTION_SIZE(section, segment)					\
  (((section->flags & (SEC_HAS_CONTENTS | SEC_THREAD_LOCAL))		\
    != SEC_THREAD_LOCAL || segment->p_type == PT_TLS)			\
   ? section->size : 0)

  /* Returns TRUE if the given section is contained within
     the given segment.  VMA addresses are compared.  */
#define IS_CONTAINED_BY_VMA(section, segment, opb)			\
  (section->vma * (opb) >= segment->p_vaddr				\
   && (section->vma * (opb) + SECTION_SIZE (section, segment)		\
       <= (SEGMENT_END (segment, segment->p_vaddr))))

  /* Returns TRUE if the given section is contained within
     the given segment.  LMA addresses are compared.  */
#define IS_CONTAINED_BY_LMA(section, segment, base, opb)		\
  (section->lma * (opb) >= base						\
   && (section->lma + SECTION_SIZE (section, segment) / (opb) >= section->lma) \
   && (section->lma * (opb) + SECTION_SIZE (section, segment)		\
       <= SEGMENT_END (segment, base)))

  /* Handle PT_NOTE segment.  */
#define IS_NOTE(p, s)							\
  (p->p_type == PT_NOTE							\
   && elf_section_type (s) == SHT_NOTE					\
   && (bfd_vma) s->filepos >= p->p_offset				\
   && ((bfd_vma) s->filepos + s->size					\
       <= p->p_offset + p->p_filesz))

  /* Special case: corefile "NOTE" section containing regs, prpsinfo
     etc.  */
#define IS_COREFILE_NOTE(p, s)						\
  (IS_NOTE (p, s)							\
   && bfd_get_format (ibfd) == bfd_core					\
   && s->vma == 0							\
   && s->lma == 0)

  ((((segment->p_paddr							\
      ? IS_CONTAINED_BY_LMA (section, segment, segment->p_paddr, opb)	\
      : IS_CONTAINED_BY_VMA (section, segment, opb))			\
    || IS_NOTE (segment, section))					\
       || SECTION_SIZE (section, segment) > 0				\
  (seg1->field >= SEGMENT_END (seg2, seg2->field))
	      extra_length = (SEGMENT_END (segment, segment->p_vaddr)
			      - SEGMENT_END (segment2, segment2->p_vaddr));
	      extra_length = (SEGMENT_END (segment2, segment2->p_vaddr)
			      - SEGMENT_END (segment, segment->p_vaddr));
	      if (IS_CONTAINED_BY_LMA (output_section, segment, map->p_paddr,
				       opb)
		  || IS_COREFILE_NOTE (segment, section)
		  || (bed->want_p_paddr_set_to_zero
		      && IS_CONTAINED_BY_VMA (output_section, segment, opb)))
	      if (IS_CONTAINED_BY_LMA (output_section, segment, map->p_paddr,
				       opb)
		  || IS_COREFILE_NOTE (segment, section))
#undef SEGMENT_END
#undef SECTION_SIZE
#undef IS_CONTAINED_BY_VMA
#undef IS_CONTAINED_BY_LMA
#undef IS_NOTE
#undef IS_COREFILE_NOTE
Fixed code: static inline bfd_vma
segment_size (Elf_Internal_Phdr *segment)
{
  return (segment->p_memsz > segment->p_filesz
	  ? segment->p_memsz : segment->p_filesz);
}


/* Returns the end address of the segment + 1.  */
static inline bfd_vma
segment_end (Elf_Internal_Phdr *segment, bfd_vma start)
{
  return start + segment_size (segment);
}

static inline bfd_size_type
section_size (asection *section, Elf_Internal_Phdr *segment)
{
  if ((section->flags & SEC_HAS_CONTENTS) != 0
      || (section->flags & SEC_THREAD_LOCAL) == 0
      || segment->p_type == PT_TLS)
    return section->size;
  return 0;
}

/* Returns TRUE if the given section is contained within the given
   segment.  LMA addresses are compared against PADDR when
   bed->want_p_paddr_set_to_zero is false, VMA against VADDR when true.  */
static bool
is_contained_by (asection *section, Elf_Internal_Phdr *segment,
		 bfd_vma paddr, bfd_vma vaddr, unsigned int opb,
		 const struct elf_backend_data *bed)
{
  bfd_vma seg_addr = !bed->want_p_paddr_set_to_zero ? paddr : vaddr;
  bfd_vma addr = !bed->want_p_paddr_set_to_zero ? section->lma : section->vma;
  bfd_vma octet;
  if (_bfd_mul_overflow (addr, opb, &octet))
    return false;
  /* The third and fourth lines below are testing that the section end
     address is within the segment.  It's written this way to avoid
     overflow.  Add seg_addr + section_size to both sides of the
     inequality to make it obvious.  */
  return (octet >= seg_addr
	  && segment_size (segment) >= section_size (section, segment)
	  && (octet - seg_addr
	      <= segment_size (segment) - section_size (section, segment)));
}

/* Handle PT_NOTE segment.  */
static bool
is_note (Elf_Internal_Phdr *p, asection *s)
{
  return (p->p_type == PT_NOTE
	  && elf_section_type (s) == SHT_NOTE
	  && (ufile_ptr) s->filepos >= p->p_offset
	  && p->p_filesz >= s->size
	  && ((ufile_ptr) s->filepos - p->p_offset
	      <= p->p_filesz - s->size));
}

  (((is_contained_by (section, segment, segment->p_paddr,		\
		      segment->p_vaddr, opb, bed)			\
    || is_note (segment, section))					\
       || section_size (section, segment) > 0				\
  (seg1->field >= segment_end (seg2, seg2->field))
	      extra_length = (segment_end (segment, segment->p_vaddr)
			      - segment_end (segment2, segment2->p_vaddr));
	      extra_length = (segment_end (segment2, segment2->p_vaddr)
			      - segment_end (segment, segment->p_vaddr));
	      if (is_contained_by (output_section, segment, map->p_paddr,
				   map->p_paddr + map->p_vaddr_offset, opb, bed)
		  || is_note (segment, section))
	      if (is_contained_by (output_section, segment, map->p_paddr,
				   map->p_paddr + map->p_vaddr_offset, opb, bed)
		  || is_note (segment, section))