[binutils-gdb] Lack of bounds checking in vms-alpha.c parse_module

Alan Modra amodra@sourceware.org
Mon Dec 12 08:59:39 GMT 2022


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

commit 77c225bdeb410cf60da804879ad41622f5f1aa44
Author: Alan Modra <amodra@gmail.com>
Date:   Mon Dec 12 18:28:49 2022 +1030

    Lack of bounds checking in vms-alpha.c parse_module
    
            PR 29873
            PR 29874
            PR 29875
            PR 29876
            PR 29877
            PR 29878
            PR 29879
            PR 29880
            PR 29881
            PR 29882
            PR 29883
            PR 29884
            PR 29885
            PR 29886
            PR 29887
            PR 29888
            PR 29889
            PR 29890
            PR 29891
            * vms-alpha.c (parse_module): Make length param bfd_size_type.
            Delete length == -1 checks.  Sanity check record_length.
            Sanity check DST__K_MODBEG, DST__K_RTNBEG, DST__K_RTNEND lengths.
            Sanity check DST__K_SOURCE and DST__K_LINE_NUM elements
            before accessing.
            (build_module_list): Pass dst_section size to parse_module.

Diff:
---
 bfd/vms-alpha.c | 213 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 168 insertions(+), 45 deletions(-)

diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c
index c0eb5bc5a2a..3b63259cc81 100644
--- a/bfd/vms-alpha.c
+++ b/bfd/vms-alpha.c
@@ -4345,7 +4345,7 @@ new_module (bfd *abfd)
 
 static bool
 parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
-	      int length)
+	      bfd_size_type length)
 {
   unsigned char *maxptr = ptr + length;
   unsigned char *src_ptr, *pcl_ptr;
@@ -4366,7 +4366,7 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
     return false;
   module->line_table = curr_line;
 
-  while (length == -1 || (ptr + 3) < maxptr)
+  while (ptr + 3 < maxptr)
     {
       /* The first byte is not counted in the recorded length.  */
       int rec_length = bfd_getl16 (ptr) + 1;
@@ -4374,15 +4374,19 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 
       vms_debug2 ((2, "DST record: leng %d, type %d\n", rec_length, rec_type));
 
-      if (length == -1 && rec_type == DST__K_MODEND)
+      if (rec_length > maxptr - ptr)
+	break;
+      if (rec_type == DST__K_MODEND)
 	break;
 
       switch (rec_type)
 	{
 	case DST__K_MODBEG:
+	  if (rec_length <= DST_S_B_MODBEG_NAME)
+	    break;
 	  module->name
 	    = _bfd_vms_save_counted_string (abfd, ptr + DST_S_B_MODBEG_NAME,
-					    maxptr - (ptr + DST_S_B_MODBEG_NAME));
+					    rec_length - DST_S_B_MODBEG_NAME);
 
 	  curr_pc = 0;
 	  prev_pc = 0;
@@ -4396,13 +4400,15 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 	  break;
 
 	case DST__K_RTNBEG:
+	  if (rec_length <= DST_S_B_RTNBEG_NAME)
+	    break;
 	  funcinfo = (struct funcinfo *)
 	    bfd_zalloc (abfd, sizeof (struct funcinfo));
 	  if (!funcinfo)
 	    return false;
 	  funcinfo->name
 	    = _bfd_vms_save_counted_string (abfd, ptr + DST_S_B_RTNBEG_NAME,
-					    maxptr - (ptr + DST_S_B_RTNBEG_NAME));
+					    rec_length - DST_S_B_RTNBEG_NAME);
 	  funcinfo->low = bfd_getl32 (ptr + DST_S_L_RTNBEG_ADDRESS);
 	  funcinfo->next = module->func_table;
 	  module->func_table = funcinfo;
@@ -4412,6 +4418,8 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 	  break;
 
 	case DST__K_RTNEND:
+	  if (rec_length < DST_S_L_RTNEND_SIZE + 4)
+	    break;
 	  if (!module->func_table)
 	    return false;
 	  module->func_table->high = module->func_table->low
@@ -4444,10 +4452,63 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 
 	  vms_debug2 ((3, "source info\n"));
 
-	  while (src_ptr < ptr + rec_length)
+	  while (src_ptr - ptr < rec_length)
 	    {
 	      int cmd = src_ptr[0], cmd_length, data;
 
+	      switch (cmd)
+		{
+		case DST__K_SRC_DECLFILE:
+		  if (src_ptr - ptr + DST_S_B_SRC_DF_LENGTH >= rec_length)
+		    cmd_length = 0x10000;
+		  else
+		    cmd_length = src_ptr[DST_S_B_SRC_DF_LENGTH] + 2;
+		  break;
+
+		case DST__K_SRC_DEFLINES_B:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_SRC_DEFLINES_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_SRC_INCRLNUM_B:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_SRC_SETFILE:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_SRC_SETLNUM_L:
+		  cmd_length = 5;
+		  break;
+
+		case DST__K_SRC_SETLNUM_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_SRC_SETREC_L:
+		  cmd_length = 5;
+		  break;
+
+		case DST__K_SRC_SETREC_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_SRC_FORMFEED:
+		  cmd_length = 1;
+		  break;
+
+		default:
+		  cmd_length = 2;
+		  break;
+		}
+
+	      if (src_ptr - ptr + cmd_length > rec_length)
+		break;
+
 	      switch (cmd)
 		{
 		case DST__K_SRC_DECLFILE:
@@ -4472,7 +4533,6 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 
 		    module->file_table [fileid].name = filename;
 		    module->file_table [fileid].srec = 1;
-		    cmd_length = src_ptr[DST_S_B_SRC_DF_LENGTH] + 2;
 		    vms_debug2 ((4, "DST_S_C_SRC_DECLFILE: %d, %s\n",
 				 fileid, module->file_table [fileid].name));
 		  }
@@ -4489,7 +4549,6 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 		  srec->sfile = curr_srec->sfile;
 		  curr_srec->next = srec;
 		  curr_srec = srec;
-		  cmd_length = 2;
 		  vms_debug2 ((4, "DST_S_C_SRC_DEFLINES_B: %d\n", data));
 		  break;
 
@@ -4504,14 +4563,12 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 		  srec->sfile = curr_srec->sfile;
 		  curr_srec->next = srec;
 		  curr_srec = srec;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST_S_C_SRC_DEFLINES_W: %d\n", data));
 		  break;
 
 		case DST__K_SRC_INCRLNUM_B:
 		  data = src_ptr[DST_S_B_SRC_UNSBYTE];
 		  curr_srec->line += data;
-		  cmd_length = 2;
 		  vms_debug2 ((4, "DST_S_C_SRC_INCRLNUM_B: %d\n", data));
 		  break;
 
@@ -4519,21 +4576,18 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 		  data = bfd_getl16 (src_ptr + DST_S_W_SRC_UNSWORD);
 		  curr_srec->sfile = data;
 		  curr_srec->srec = module->file_table[data].srec;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST_S_C_SRC_SETFILE: %d\n", data));
 		  break;
 
 		case DST__K_SRC_SETLNUM_L:
 		  data = bfd_getl32 (src_ptr + DST_S_L_SRC_UNSLONG);
 		  curr_srec->line = data;
-		  cmd_length = 5;
 		  vms_debug2 ((4, "DST_S_C_SRC_SETLNUM_L: %d\n", data));
 		  break;
 
 		case DST__K_SRC_SETLNUM_W:
 		  data = bfd_getl16 (src_ptr + DST_S_W_SRC_UNSWORD);
 		  curr_srec->line = data;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST_S_C_SRC_SETLNUM_W: %d\n", data));
 		  break;
 
@@ -4541,7 +4595,6 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 		  data = bfd_getl32 (src_ptr + DST_S_L_SRC_UNSLONG);
 		  curr_srec->srec = data;
 		  module->file_table[curr_srec->sfile].srec = data;
-		  cmd_length = 5;
 		  vms_debug2 ((4, "DST_S_C_SRC_SETREC_L: %d\n", data));
 		  break;
 
@@ -4549,19 +4602,16 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 		  data = bfd_getl16 (src_ptr + DST_S_W_SRC_UNSWORD);
 		  curr_srec->srec = data;
 		  module->file_table[curr_srec->sfile].srec = data;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST_S_C_SRC_SETREC_W: %d\n", data));
 		  break;
 
 		case DST__K_SRC_FORMFEED:
-		  cmd_length = 1;
 		  vms_debug2 ((4, "DST_S_C_SRC_FORMFEED\n"));
 		  break;
 
 		default:
 		  _bfd_error_handler (_("unknown source command %d"),
 				      cmd);
-		  cmd_length = 2;
 		  break;
 		}
 
@@ -4574,18 +4624,114 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 
 	  vms_debug2 ((3, "line info\n"));
 
-	  while (pcl_ptr < ptr + rec_length)
+	  while (pcl_ptr - ptr < rec_length)
 	    {
 	      /* The command byte is signed so we must sign-extend it.  */
 	      int cmd = ((signed char *)pcl_ptr)[0], cmd_length, data;
 
+	      switch (cmd)
+		{
+		case DST__K_DELTA_PC_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_DELTA_PC_L:
+		  cmd_length = 5;
+		  break;
+
+		case DST__K_INCR_LINUM:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_INCR_LINUM_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_INCR_LINUM_L:
+		  cmd_length = 5;
+		  break;
+
+		case DST__K_SET_LINUM_INCR:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_SET_LINUM_INCR_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_RESET_LINUM_INCR:
+		  cmd_length = 1;
+		  break;
+
+		case DST__K_BEG_STMT_MODE:
+		  cmd_length = 1;
+		  break;
+
+		case DST__K_END_STMT_MODE:
+		  cmd_length = 1;
+		  break;
+
+		case DST__K_SET_LINUM_B:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_SET_LINUM:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_SET_LINUM_L:
+		  cmd_length = 5;
+		  break;
+
+		case DST__K_SET_PC:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_SET_PC_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_SET_PC_L:
+		  cmd_length = 5;
+		  break;
+
+		case DST__K_SET_STMTNUM:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_TERM:
+		  cmd_length = 2;
+		  break;
+
+		case DST__K_TERM_W:
+		  cmd_length = 3;
+		  break;
+
+		case DST__K_TERM_L:
+		  cmd_length = 5;
+		  break;
+
+		case DST__K_SET_ABS_PC:
+		  cmd_length = 5;
+		  break;
+
+		default:
+		  if (cmd <= 0)
+		    cmd_length = 1;
+		  else
+		    cmd_length = 2;
+		  break;
+		}
+
+	      if (pcl_ptr - ptr + cmd_length > rec_length)
+		break;
+
 	      switch (cmd)
 		{
 		case DST__K_DELTA_PC_W:
 		  data = bfd_getl16 (pcl_ptr + DST_S_W_PCLINE_UNSWORD);
 		  curr_pc += data;
 		  curr_linenum += 1;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST__K_DELTA_PC_W: %d\n", data));
 		  break;
 
@@ -4593,131 +4739,111 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 		  data = bfd_getl32 (pcl_ptr + DST_S_L_PCLINE_UNSLONG);
 		  curr_pc += data;
 		  curr_linenum += 1;
-		  cmd_length = 5;
 		  vms_debug2 ((4, "DST__K_DELTA_PC_L: %d\n", data));
 		  break;
 
 		case DST__K_INCR_LINUM:
 		  data = pcl_ptr[DST_S_B_PCLINE_UNSBYTE];
 		  curr_linenum += data;
-		  cmd_length = 2;
 		  vms_debug2 ((4, "DST__K_INCR_LINUM: %d\n", data));
 		  break;
 
 		case DST__K_INCR_LINUM_W:
 		  data = bfd_getl16 (pcl_ptr + DST_S_W_PCLINE_UNSWORD);
 		  curr_linenum += data;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST__K_INCR_LINUM_W: %d\n", data));
 		  break;
 
 		case DST__K_INCR_LINUM_L:
 		  data = bfd_getl32 (pcl_ptr + DST_S_L_PCLINE_UNSLONG);
 		  curr_linenum += data;
-		  cmd_length = 5;
 		  vms_debug2 ((4, "DST__K_INCR_LINUM_L: %d\n", data));
 		  break;
 
 		case DST__K_SET_LINUM_INCR:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_SET_LINUM_INCR");
-		  cmd_length = 2;
 		  break;
 
 		case DST__K_SET_LINUM_INCR_W:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_SET_LINUM_INCR_W");
-		  cmd_length = 3;
 		  break;
 
 		case DST__K_RESET_LINUM_INCR:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_RESET_LINUM_INCR");
-		  cmd_length = 1;
 		  break;
 
 		case DST__K_BEG_STMT_MODE:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_BEG_STMT_MODE");
-		  cmd_length = 1;
 		  break;
 
 		case DST__K_END_STMT_MODE:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_END_STMT_MODE");
-		  cmd_length = 1;
 		  break;
 
 		case DST__K_SET_LINUM_B:
 		  data = pcl_ptr[DST_S_B_PCLINE_UNSBYTE];
 		  curr_linenum = data;
-		  cmd_length = 2;
 		  vms_debug2 ((4, "DST__K_SET_LINUM_B: %d\n", data));
 		  break;
 
 		case DST__K_SET_LINUM:
 		  data = bfd_getl16 (pcl_ptr + DST_S_W_PCLINE_UNSWORD);
 		  curr_linenum = data;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST__K_SET_LINE_NUM: %d\n", data));
 		  break;
 
 		case DST__K_SET_LINUM_L:
 		  data = bfd_getl32 (pcl_ptr + DST_S_L_PCLINE_UNSLONG);
 		  curr_linenum = data;
-		  cmd_length = 5;
 		  vms_debug2 ((4, "DST__K_SET_LINUM_L: %d\n", data));
 		  break;
 
 		case DST__K_SET_PC:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_SET_PC");
-		  cmd_length = 2;
 		  break;
 
 		case DST__K_SET_PC_W:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_SET_PC_W");
-		  cmd_length = 3;
 		  break;
 
 		case DST__K_SET_PC_L:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_SET_PC_L");
-		  cmd_length = 5;
 		  break;
 
 		case DST__K_SET_STMTNUM:
 		  _bfd_error_handler
 		    (_("%s not implemented"), "DST__K_SET_STMTNUM");
-		  cmd_length = 2;
 		  break;
 
 		case DST__K_TERM:
 		  data = pcl_ptr[DST_S_B_PCLINE_UNSBYTE];
 		  curr_pc += data;
-		  cmd_length = 2;
 		  vms_debug2 ((4, "DST__K_TERM: %d\n", data));
 		  break;
 
 		case DST__K_TERM_W:
 		  data = bfd_getl16 (pcl_ptr + DST_S_W_PCLINE_UNSWORD);
 		  curr_pc += data;
-		  cmd_length = 3;
 		  vms_debug2 ((4, "DST__K_TERM_W: %d\n", data));
 		  break;
 
 		case DST__K_TERM_L:
 		  data = bfd_getl32 (pcl_ptr + DST_S_L_PCLINE_UNSLONG);
 		  curr_pc += data;
-		  cmd_length = 5;
 		  vms_debug2 ((4, "DST__K_TERM_L: %d\n", data));
 		  break;
 
 		case DST__K_SET_ABS_PC:
 		  data = bfd_getl32 (pcl_ptr + DST_S_L_PCLINE_UNSLONG);
 		  curr_pc = data;
-		  cmd_length = 5;
 		  vms_debug2 ((4, "DST__K_SET_ABS_PC: 0x%x\n", data));
 		  break;
 
@@ -4726,15 +4852,11 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr,
 		    {
 		      curr_pc -= cmd;
 		      curr_linenum += 1;
-		      cmd_length = 1;
 		      vms_debug2 ((4, "bump pc to 0x%lx and line to %d\n",
 				   (unsigned long)curr_pc, curr_linenum));
 		    }
 		  else
-		    {
-		      _bfd_error_handler (_("unknown line command %d"), cmd);
-		      cmd_length = 2;
-		    }
+		    _bfd_error_handler (_("unknown line command %d"), cmd);
 		  break;
 		}
 
@@ -4864,7 +4986,8 @@ build_module_list (bfd *abfd)
 	return NULL;
 
       module = new_module (abfd);
-      if (!parse_module (abfd, module, PRIV (dst_section)->contents, -1))
+      if (!parse_module (abfd, module, PRIV (dst_section)->contents,
+			 PRIV (dst_section)->size))
 	return NULL;
       list = module;
     }


More information about the Binutils-cvs mailing list