From f6389c5a793648f1b12cc791b8957cf6d1752222 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Mon, 27 Feb 2023 14:53:22 +1030 Subject: [PATCH] Add some sanity checking in ECOFF lookup_line More anti-fuzzer bounds checking for the ECOFF support. A lot of this is in ancient code using "long" for counts and sizes, which is why the patch uses "(long) ((unsigned long) x + 1) > 0" in a few places. The unsigned long cast is so that "x + 1" doesn't trigger ubsan warnings about signed integer overflow. It would be a good idea to replace most of the longs used in binutils with size_t, but that's more than I care to do for COFF/ECOFF. * ecofflink.c (mk_fdrtab): Sanity check string offsets. (lookup_line): Likewise, and symbol indices. --- bfd/ecofflink.c | 146 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 102 insertions(+), 44 deletions(-) diff --git a/bfd/ecofflink.c b/bfd/ecofflink.c index e902bd51d53..422ce57f430 100644 --- a/bfd/ecofflink.c +++ b/bfd/ecofflink.c @@ -1782,8 +1782,13 @@ mk_fdrtab (bfd *abfd, sym_ptr = ((char *) debug_info->external_sym + (fdr_ptr->isymBase + 1) * debug_swap->external_sym_size); (*debug_swap->swap_sym_in) (abfd, sym_ptr, &sym); - if (strcmp (debug_info->ss + fdr_ptr->issBase + sym.iss, - STABS_SYMBOL) == 0) + if (fdr_ptr->issBase >= 0 + && fdr_ptr->issBase < debug_info->symbolic_header.issMax + && sym.iss >= 0 + && sym.iss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase) + && strcmp (debug_info->ss + fdr_ptr->issBase + sym.iss, + STABS_SYMBOL) == 0) stabs = true; } @@ -1981,14 +1986,27 @@ lookup_line (bfd *abfd, char *sym_ptr; SYMR sym; - sym_ptr = ((char *) debug_info->external_sym - + (fdr_ptr->isymBase + 1) * debug_swap->external_sym_size); - (*debug_swap->swap_sym_in) (abfd, sym_ptr, &sym); - if (strcmp (debug_info->ss + fdr_ptr->issBase + sym.iss, - STABS_SYMBOL) == 0) - stabs = true; + if ((long) ((unsigned long) fdr_ptr->isymBase + 1) > 0 + && fdr_ptr->isymBase + 1 < debug_info->symbolic_header.isymMax) + { + sym_ptr = ((char *) debug_info->external_sym + + (fdr_ptr->isymBase + 1) * debug_swap->external_sym_size); + (*debug_swap->swap_sym_in) (abfd, sym_ptr, &sym); + if (fdr_ptr->issBase >= 0 + && fdr_ptr->issBase < debug_info->symbolic_header.issMax + && sym.iss >= 0 + && sym.iss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase) + && strcmp (debug_info->ss + fdr_ptr->issBase + sym.iss, + STABS_SYMBOL) == 0) + stabs = true; + } } + line_info->cache.filename = NULL; + line_info->cache.functionname = NULL; + line_info->cache.line_num = 0; + if (!stabs) { bfd_size_type external_pdr_size; @@ -2167,38 +2185,50 @@ lookup_line (bfd *abfd, symbols, at least according to gdb/mipsread.c. */ if (fdr_ptr->rss == -1) { - line_info->cache.filename = NULL; - if (pdr.isym == -1) - line_info->cache.functionname = NULL; - else - { - EXTR proc_ext; + EXTR proc_ext; + if (pdr.isym >= 0 + && pdr.isym < debug_info->symbolic_header.iextMax) + { (*debug_swap->swap_ext_in) - (abfd, - ((char *) debug_info->external_ext - + pdr.isym * debug_swap->external_ext_size), + (abfd, ((char *) debug_info->external_ext + + pdr.isym * debug_swap->external_ext_size), &proc_ext); - line_info->cache.functionname = (debug_info->ssext - + proc_ext.asym.iss); + if (proc_ext.asym.iss >= 0 + && proc_ext.asym.iss < debug_info->symbolic_header.issExtMax) + line_info->cache.functionname = (debug_info->ssext + + proc_ext.asym.iss); } } - else + else if (fdr_ptr->issBase >= 0 + && fdr_ptr->issBase < debug_info->symbolic_header.issMax + && fdr_ptr->rss >= 0 + && fdr_ptr->rss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase)) { SYMR proc_sym; line_info->cache.filename = (debug_info->ss + fdr_ptr->issBase + fdr_ptr->rss); - (*debug_swap->swap_sym_in) - (abfd, - ((char *) debug_info->external_sym - + ((fdr_ptr->isymBase + pdr.isym) - * debug_swap->external_sym_size)), - &proc_sym); - line_info->cache.functionname = (debug_info->ss - + fdr_ptr->issBase - + proc_sym.iss); + if (fdr_ptr->isymBase >= 0 + && fdr_ptr->isymBase < debug_info->symbolic_header.isymMax + && pdr.isym >= 0 + && pdr.isym < (debug_info->symbolic_header.isymMax + - fdr_ptr->isymBase)) + { + (*debug_swap->swap_sym_in) + (abfd, ((char *) debug_info->external_sym + + ((fdr_ptr->isymBase + pdr.isym) + * debug_swap->external_sym_size)), + &proc_sym); + if (proc_sym.iss >= 0 + && proc_sym.iss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase)) + line_info->cache.functionname = (debug_info->ss + + fdr_ptr->issBase + + proc_sym.iss); + } } if (lineno == ilineNil) lineno = 0; @@ -2230,10 +2260,6 @@ lookup_line (bfd *abfd, looking through the symbols until we find both a line number and a function name which are beyond the address we want. */ - line_info->cache.filename = NULL; - line_info->cache.functionname = NULL; - line_info->cache.line_num = 0; - directory_name = NULL; main_file_name = NULL; current_file_name = NULL; @@ -2246,9 +2272,21 @@ lookup_line (bfd *abfd, external_sym_size = debug_swap->external_sym_size; - sym_ptr = ((char *) debug_info->external_sym - + (fdr_ptr->isymBase + 2) * external_sym_size); - sym_ptr_end = sym_ptr + (fdr_ptr->csym - 2) * external_sym_size; + if (fdr_ptr->isymBase >= 0 + && fdr_ptr->isymBase < debug_info->symbolic_header.isymMax + && fdr_ptr->csym >= 2 + && fdr_ptr->csym < (debug_info->symbolic_header.isymMax + - fdr_ptr->isymBase)) + { + sym_ptr = ((char *) debug_info->external_sym + + (fdr_ptr->isymBase + 2) * external_sym_size); + sym_ptr_end = sym_ptr + (fdr_ptr->csym - 2) * external_sym_size; + } + else + { + sym_ptr = NULL; + sym_ptr_end = sym_ptr; + } for (; sym_ptr < sym_ptr_end && (! past_line || ! past_fn); sym_ptr += external_sym_size) @@ -2262,8 +2300,13 @@ lookup_line (bfd *abfd, switch (ECOFF_UNMARK_STAB (sym.index)) { case N_SO: - main_file_name = current_file_name = - debug_info->ss + fdr_ptr->issBase + sym.iss; + if (fdr_ptr->issBase >= 0 + && fdr_ptr->issBase < debug_info->symbolic_header.issMax + && sym.iss >= 0 + && sym.iss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase)) + main_file_name = current_file_name + = debug_info->ss + fdr_ptr->issBase + sym.iss; /* Check the next symbol to see if it is also an N_SO symbol. */ @@ -2278,16 +2321,26 @@ lookup_line (bfd *abfd, && ECOFF_UNMARK_STAB (nextsym.index) == N_SO) { directory_name = current_file_name; - main_file_name = current_file_name = - debug_info->ss + fdr_ptr->issBase + nextsym.iss; + if (fdr_ptr->issBase >= 0 + && fdr_ptr->issBase < debug_info->symbolic_header.issMax + && nextsym.iss >= 0 + && nextsym.iss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase)) + main_file_name = current_file_name + = debug_info->ss + fdr_ptr->issBase + nextsym.iss; sym_ptr += external_sym_size; } } break; case N_SOL: - current_file_name = - debug_info->ss + fdr_ptr->issBase + sym.iss; + if (fdr_ptr->issBase >= 0 + && fdr_ptr->issBase < debug_info->symbolic_header.issMax + && sym.iss >= 0 + && sym.iss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase)) + current_file_name + = debug_info->ss + fdr_ptr->issBase + sym.iss; break; case N_FUN: @@ -2296,8 +2349,13 @@ lookup_line (bfd *abfd, else if (sym.value >= low_func_vma) { low_func_vma = sym.value; - function_name = - debug_info->ss + fdr_ptr->issBase + sym.iss; + if (fdr_ptr->issBase >= 0 + && fdr_ptr->issBase < debug_info->symbolic_header.issMax + && sym.iss >= 0 + && sym.iss < (debug_info->symbolic_header.issMax + - fdr_ptr->issBase)) + function_name + = debug_info->ss + fdr_ptr->issBase + sym.iss; } break; } -- 2.30.2