On Sat, Jul 30, 2022 at 07:29:23AM +, 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
> 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();
>
> + /* 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()) != '"')
> - if (ch != '\\')
> - out_schar (ch);
> - else
> - {
> - ch = byte();
> - 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();
> + 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 (_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
