[binutils-gdb] Buffer overflow in evax_bfd_print_eobj

[binutils-gdb] Buffer overflow in evax_bfd_print_eobj

[Alan Modra]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0ea8d78bd3ff77499e9d7c19d7a40cea680e89de

commit 0ea8d78bd3ff77499e9d7c19d7a40cea680e89de
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Feb 18 14:31:45 2023 +1030

    Buffer overflow in evax_bfd_print_eobj
    
            * vms-alpha.c (evax_bfd_print_eobj): Rewrite header handling,
            sanity checking rec_len.  Check bfd_malloc return.

Diff:
---
 bfd/vms-alpha.c | 111 ++++++++++++++++++++++++--------------------------------
 1 file changed, 48 insertions(+), 63 deletions(-)

diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c
index fd2cf50441f..d06d743f224 100644
--- a/bfd/vms-alpha.c
+++ b/bfd/vms-alpha.c
@@ -6853,7 +6853,7 @@ static void
 evax_bfd_print_eobj (struct bfd *abfd, FILE *file)
 {
   bool is_first = true;
-  bool has_records = false;
+  bool has_records = true;
 
   while (1)
     {
@@ -6862,81 +6862,68 @@ evax_bfd_print_eobj (struct bfd *abfd, FILE *file)
       unsigned char *rec;
       unsigned int hdr_size;
       unsigned int type;
+      unsigned char buf[6];
 
-      if (is_first)
+      hdr_size = has_records ? 6 : 4;
+      if (bfd_bread (buf, hdr_size, abfd) != hdr_size)
 	{
-	  unsigned char buf[6];
-
-	  is_first = false;
-
-	  /* Read 6 bytes.  */
-	  if (bfd_bread (buf, sizeof (buf), abfd) != sizeof (buf))
-	    {
-	      fprintf (file, _("cannot read GST record length\n"));
-	      return;
-	    }
-	  rec_len = bfd_getl16 (buf + 0);
-	  if (rec_len == bfd_getl16 (buf + 4)
-	      && bfd_getl16 (buf + 2) == EOBJ__C_EMH)
-	    {
-	      /* The format is raw: record-size, type, record-size.  */
-	      has_records = true;
-	      pad_len = (rec_len + 1) & ~1U;
-	      hdr_size = 4;
-	    }
-	  else if (rec_len == EOBJ__C_EMH)
-	    {
-	      has_records = false;
-	      pad_len = bfd_getl16 (buf + 2);
-	      hdr_size = 6;
-	    }
-	  else
-	    {
-	      /* Ill-formed.  */
-	      fprintf (file, _("cannot find EMH in first GST record\n"));
-	      return;
-	    }
-	  rec = bfd_malloc (pad_len);
-	  memcpy (rec, buf + sizeof (buf) - hdr_size, hdr_size);
+	  fprintf (file, _("cannot read GST record header\n"));
+	  return;
 	}
-      else
+
+      type = bfd_getl16 (buf);
+      rec_len = bfd_getl16 (buf + 2);
+      pad_len = rec_len;
+      if (has_records)
 	{
-	  unsigned int rec_len2 = 0;
-	  unsigned char hdr[4];
+	  unsigned int rec_len2 = bfd_getl16 (buf + 4);
 
-	  if (has_records)
+	  if (is_first)
 	    {
-	      unsigned char buf_len[2];
-
-	      if (bfd_bread (buf_len, sizeof (buf_len), abfd)
-		  != sizeof (buf_len))
+	      is_first = false;
+	      if (type == rec_len2 && rec_len == EOBJ__C_EMH)
+		/* Matched a VMS record EMH.  */
+		;
+	      else
 		{
-		  fprintf (file, _("cannot read GST record length\n"));
-		  return;
+		  has_records = false;
+		  if (type != EOBJ__C_EMH)
+		    {
+		      /* Ill-formed.  */
+		      fprintf (file, _("cannot find EMH in first GST record\n"));
+		      return;
+		    }
 		}
-	      rec_len2 = (unsigned)bfd_getl16 (buf_len);
 	    }
 
-	  if (bfd_bread (hdr, sizeof (hdr), abfd) != sizeof (hdr))
-	    {
-	      fprintf (file, _("cannot read GST record header\n"));
-	      return;
-	    }
-	  rec_len = (unsigned)bfd_getl16 (hdr + 2);
 	  if (has_records)
-	    pad_len = (rec_len + 1) & ~1U;
-	  else
-	    pad_len = rec_len;
-	  rec = bfd_malloc (pad_len);
-	  memcpy (rec, hdr, sizeof (hdr));
-	  hdr_size = sizeof (hdr);
-	  if (has_records && rec_len2 != rec_len)
 	    {
-	      fprintf (file, _(" corrupted GST\n"));
-	      break;
+	      /* VMS record format is: record-size, type, record-size.
+		 See maybe_adjust_record_pointer_for_object comment.  */
+	      if (type == rec_len2)
+		{
+		  type = rec_len;
+		  rec_len = rec_len2;
+		}
+	      else
+		rec_len = 0;
+	      pad_len = (rec_len + 1) & ~1U;
+	      hdr_size = 4;
 	    }
 	}
 
+      if (rec_len < hdr_size)
+	{
+	  fprintf (file, _("corrupted GST\n"));
+	  return;
+	}
+
+      rec = bfd_malloc (pad_len);
+      if (rec == NULL)
+	return;
+
+      memcpy (rec, buf + (has_records ? 2 : 0), hdr_size);
+
       if (bfd_bread (rec + hdr_size, pad_len - hdr_size, abfd)
 	  != pad_len - hdr_size)
 	{
@@ -6944,8 +6931,6 @@ evax_bfd_print_eobj (struct bfd *abfd, FILE *file)
 	  return;
 	}
 
-      type = (unsigned)bfd_getl16 (rec);
-
       switch (type)
 	{
 	case EOBJ__C_EMH: