[binutils-gdb] buffer_read_memory hardening

Alan Modra via Binutils-cvs <[email protected]> Wed, 17 Jun 2026 23:56:05 +0000 (GMT)
Newsgroups gmane.comp.gnu.binutils.cvs
Message-ID <[email protected]>
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b81d60ecd1b5ffadd80f51d0d387de3762f47b46

commit b81d60ecd1b5ffadd80f51d0d387de3762f47b46
Author: Alan Modra <[email protected]>
Date:   Thu Jun 18 09:22:32 2026 +0930

    buffer_read_memory hardening
    
    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:
---
 opcodes/dis-buf.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

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;
 }