----- Forwarded message from SDA <marathon.duran...@gmail.com> ----- Date: Thu, 4 Aug 2022 20:37:47 -0400 From: SDA <marathon.duran...@gmail.com> Subject: Re: Unsolicited GNU bc patch To: debian-user@lists.debian.org
On Sat, Jul 30, 2022 at 07:29:23AM +0000, Thomas DiModica wrote: Hi, Thomas. I'd send this to the debian-developer email list rather than this user list. Not sure if any developers are lurking here ... > Greetings, > > I don't remember how or why I initially stumbled across this bug report > (https://bugs.launchpad.net/ubuntu/+source/bc/+bug/1775776), but, given that > I have some familiarity with GNU bc, I decided to fix some of the issues. > Turns out, this also seems to fix the crashes reported here > (https://www.openwall.com/lists/oss-security/2018/11/28/1). I think it would > be a lot more useful to share this, as there isn't a lot to review. There are > three bug fixes and some self-defensive checks in the runtime for malformed > bytecode. Address Sanitizer tells me that these previously invalid memory > references now just leak memory. I don't appear to have broken anything in > the process, either. > > Just trying to be somewhat helpful, > Thomas DiModica > From 3ecfe21c965956f3913e9bc340df729234e4453b Mon Sep 17 00:00:00 2001 > From: Thomas DiModica <ricinw...@yahoo.com> > Date: Tue, 19 Jul 2022 19:28:12 -0600 > Subject: [PATCH] Resolving the crashes found through fuzz testing by > HongxuChen. > > --- > bc/execute.c | 54 +++++++++++++++++++++++++++++++++------------------- > bc/storage.c | 38 ++++++++++++++++++++++++++++++++++-- > bc/util.c | 2 +- > 3 files changed, 71 insertions(+), 23 deletions(-) > > diff --git a/bc/execute.c b/bc/execute.c > index 256e4b7..d30c6f5 100644 > --- a/bc/execute.c > +++ b/bc/execute.c > @@ -130,7 +130,7 @@ execute (void) > gp = functions[pc.pc_func].f_label; > l_gp = label_num >> BC_LABEL_LOG; > l_off = label_num % BC_LABEL_GROUP; > - while (l_gp-- > 0) gp = gp->l_next; > + while ((l_gp-- > 0) && (gp != NULL)) gp = gp->l_next; > if (gp) > pc.pc_addr = gp->l_adrs[l_off]; > else { > @@ -146,6 +146,13 @@ execute (void) > if ((new_func & 0x80) != 0) > new_func = ((new_func & 0x7f) << 8) + byte(&pc); > > + /* Check to make sure it is valid. */ > + if (new_func >= f_count) > + { > + rt_error ("Internal error."); > + break; > + } > + > /* Check to make sure it is defined. */ > if (!functions[new_func].f_defined) > { > @@ -204,25 +211,32 @@ execute (void) > > case 'O' : /* Write a string to the output with processing. */ > while ((ch = byte(&pc)) != '"') > - if (ch != '\\') > - out_schar (ch); > - else > - { > - ch = byte(&pc); > - if (ch == '"') break; > - switch (ch) > - { > - case 'a': out_schar (007); break; > - case 'b': out_schar ('\b'); break; > - case 'f': out_schar ('\f'); break; > - case 'n': out_schar ('\n'); break; > - case 'q': out_schar ('"'); break; > - case 'r': out_schar ('\r'); break; > - case 't': out_schar ('\t'); break; > - case '\\': out_schar ('\\'); break; > - default: break; > - } > - } > + { > + if (pc.pc_addr == functions[pc.pc_func].f_code_size) > + { > + rt_error ("Broken String."); > + break; > + } > + if (ch != '\\') > + out_schar (ch); > + else > + { > + ch = byte(&pc); > + if (ch == '"') break; > + switch (ch) > + { > + case 'a': out_schar (007); break; > + case 'b': out_schar ('\b'); break; > + case 'f': out_schar ('\f'); break; > + case 'n': out_schar ('\n'); break; > + case 'q': out_schar ('"'); break; > + case 'r': out_schar ('\r'); break; > + case 't': out_schar ('\t'); break; > + case '\\': out_schar ('\\'); break; > + default: break; > + } > + } > + } > fflush (stdout); > break; > > diff --git a/bc/storage.c b/bc/storage.c > index c79db82..28e933b 100644 > --- a/bc/storage.c > +++ b/bc/storage.c > @@ -349,6 +349,7 @@ get_var (int var_name) > { > var_ptr = variables[var_name] = bc_malloc (sizeof (bc_var)); > bc_init_num (&var_ptr->v_value); > + var_ptr->v_next = NULL; > } > return var_ptr; > } > @@ -370,6 +371,12 @@ get_array_num (int var_index, unsigned long idx) > unsigned int ix, ix1; > int sub [NODE_DEPTH]; > > + if (var_index >= a_count) > + { > + rt_error ("Internal Error."); > + return NULL; > + } > + > /* Get the array entry. */ > ary_ptr = arrays[var_index]; > if (ary_ptr == NULL) > @@ -588,6 +595,12 @@ store_array (int var_name) > bc_num *num_ptr; > long idx; > > + if (var_name >= a_count) > + { > + rt_error ("Internal Error."); > + return; > + } > + > if (!check_stack(2)) return; > idx = bc_num2long (ex_stack->s_next->s_num); > if (idx < 0 || idx > BC_DIM_MAX || > @@ -666,6 +679,12 @@ load_array (int var_name) > bc_num *num_ptr; > long idx; > > + if (var_name >= a_count) > + { > + rt_error ("Internal Error."); > + return; > + } > + > if (!check_stack(1)) return; > idx = bc_num2long (ex_stack->s_num); > if (idx < 0 || idx > BC_DIM_MAX || > @@ -746,6 +765,12 @@ decr_array (int var_name) > bc_num *num_ptr; > long idx; > > + if (var_name >= a_count) > + { > + rt_error ("Internal Error."); > + return; > + } > + > /* It is an array variable. */ > if (!check_stack (1)) return; > idx = bc_num2long (ex_stack->s_num); > @@ -828,6 +853,12 @@ incr_array (int var_name) > bc_num *num_ptr; > long idx; > > + if (var_name >= a_count) > + { > + rt_error ("Internal Error."); > + return; > + } > + > if (!check_stack (1)) return; > idx = bc_num2long (ex_stack->s_num); > if (idx < 0 || idx > BC_DIM_MAX || > @@ -1018,7 +1049,11 @@ process_params (program_counter *progctr, int func) > > /* Compute source index and make sure some structure exists. */ > ix = (int) bc_num2long (ex_stack->s_num); > - (void) get_array_num (ix, 0); > + if (get_array_num (ix, 0) == NULL) > + { > + rt_error ("Internal Error."); > + return; > + } > > /* Push a new array and Compute Destination index */ > auto_var (params->av_name); > @@ -1049,7 +1084,6 @@ process_params (program_counter *progctr, int func) > else > rt_error ("Parameter type mismatch, parameter %s.", > v_names[params->av_name]); > - params++; > } > pop (); > } > diff --git a/bc/util.c b/bc/util.c > index 8eba093..5ff93e0 100644 > --- a/bc/util.c > +++ b/bc/util.c > @@ -610,7 +610,7 @@ lookup (char *name, int namekind) > { > if (id->v_name >= v_count) > more_variables (); > - v_names[id->v_name - 1] = name; > + v_names[id->v_name] = name; > return (id->v_name); > } > yyerror ("Too many variables"); > -- > 2.35.1.windows.2 > ----- End forwarded message -----
