buffer_read_memory hardening

Alan Modra amodra@gmail.com
Wed Jun 17 23:55:26 GMT 2026


What drew my attention here was wondering if division of the buffer
length or the requested length by octets_per_byte could result in
trucation and thus an incomplete check for possible buffer overflow.
Rather than convincing myself that this couldn't happen, I decided to
rewrite the checks in a way that avoids the possibility of truncation,
and avoids any addition overflows too.

	* dis-buf.c (buffer_read_memory): Rewrite buffer overflow
	sanity checks.

diff --git a/opcodes/dis-buf.c b/opcodes/dis-buf.c
index 83fce0a2fc3..504d0b37b98 100644
--- a/opcodes/dis-buf.c
+++ b/opcodes/dis-buf.c
@@ -32,18 +32,19 @@ buffer_read_memory (bfd_vma memaddr,
 		    struct disassemble_info *info)
 {
   unsigned int opb = info->octets_per_byte;
-  size_t end_addr_offset = length / opb;
-  size_t max_addr_offset = info->buffer_length / opb;
-  size_t octets = (memaddr - info->buffer_vma) * opb;
+  size_t addr_off, to_stop;
 
   if (memaddr < info->buffer_vma
-      || memaddr - info->buffer_vma > max_addr_offset
-      || memaddr - info->buffer_vma + end_addr_offset > max_addr_offset
-      || (info->stop_vma && (memaddr >= info->stop_vma
-			     || memaddr + end_addr_offset > info->stop_vma)))
+      || _bfd_mul_overflow (memaddr - info->buffer_vma, opb, &addr_off)
+      || addr_off > info->buffer_length
+      || length > info->buffer_length - addr_off
+      || (info->stop_vma
+	  && (memaddr >= info->stop_vma
+	      || _bfd_mul_overflow (info->stop_vma - memaddr, opb, &to_stop)
+	      || length > to_stop)))
     /* Out of bounds.  Use EIO because GDB uses it.  */
     return EIO;
-  memcpy (myaddr, info->buffer + octets, length);
+  memcpy (myaddr, info->buffer + addr_off, length);
 
   return 0;
 }

-- 
Alan Modra


More information about the Binutils mailing list