On Mon, Nov 24, 2014 at 01:38:40PM +0100, S??bastien Marie wrote:
> Hi,
>
> Starting to play with afl-fuzz, I test it with dc(1), and it found a "Bus
> error".
>
> Basically:
> $ echo '1 2:x1Lx1:x1:x' | dc
> Bus error (core dumped)
>
> I traced the bug, and the code before do a double-free (resulting the
> Bus error). Thanks to malloc(3) junk :)
>
> The problem is a lack of initialisation in stack_pushnumber function.
>
> The code before will:
> - allocate an array on register (call it A)
> A[1] = 2
>
> - push it on the stack
> (the array A arrive on stack)
>
> - store it in a new array on register (call it B)
> (the stack is just reduced, the value.array is keeped as it, so
> address of A is here)
> B[1] = A
>
> - push a number on stack
> (as the value isn't properly reinitialized, the number is allocated,
> but the value.array A is keeped)
>
> - allocate an array on register
> - it pops the number (with value.array setted to A), and as it pops,
> it free the value on stack, and the array A too (as != NULL)
> - it pops the array B
> - it will try to set B[1] = 1, so it free B[1], which is A, which is
> already freed: *boom*.
>
> The patch just ensure a push_number (or push_string) properly initialize
> the value, by set value.array to NULL.
> --
> S??bastien Marie
>
> Index: stack.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/dc/stack.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 stack.c
> --- stack.c 27 Oct 2009 23:59:37 -0000 1.11
> +++ stack.c 24 Nov 2014 12:31:53 -0000
> @@ -147,6 +147,7 @@ stack_pushnumber(struct stack *stack, st
> stack_grow(stack);
> stack->stack[stack->sp].type = BCODE_NUMBER;
> stack->stack[stack->sp].u.num = b;
> + stack->stack[stack->sp].array = NULL;
> }
>
> void
> @@ -155,6 +156,7 @@ stack_pushstring(struct stack *stack, ch
> stack_grow(stack);
> stack->stack[stack->sp].type = BCODE_STRING;
> stack->stack[stack->sp].u.string = string;
> + stack->stack[stack->sp].array = NULL;
> }
>
> void
Actually, with this, the init in stack_grow can be left out, I believe.
Regress tests still succeed. I wil be adding you case to them.
-Otto
Index: stack.c
===================================================================
RCS file: /cvs/src/usr.bin/dc/stack.c,v
retrieving revision 1.11
diff -u -p -r1.11 stack.c
--- stack.c 27 Oct 2009 23:59:37 -0000 1.11
+++ stack.c 24 Nov 2014 16:57:09 -0000
@@ -129,14 +129,12 @@ stack_swap(struct stack *stack)
static void
stack_grow(struct stack *stack)
{
- size_t new_size, i;
+ size_t new_size;
if (++stack->sp == stack->size) {
new_size = stack->size * 2 + 1;
stack->stack = brealloc(stack->stack,
new_size * sizeof(*stack->stack));
- for (i = stack->size; i < new_size; i++)
- stack->stack[i].array = NULL;
stack->size = new_size;
}
}
@@ -147,6 +145,7 @@ stack_pushnumber(struct stack *stack, st
stack_grow(stack);
stack->stack[stack->sp].type = BCODE_NUMBER;
stack->stack[stack->sp].u.num = b;
+ stack->stack[stack->sp].array = NULL;
}
void
@@ -155,6 +154,7 @@ stack_pushstring(struct stack *stack, ch
stack_grow(stack);
stack->stack[stack->sp].type = BCODE_STRING;
stack->stack[stack->sp].u.string = string;
+ stack->stack[stack->sp].array = NULL;
}
void
