Been meaning to file this one for a while now. Basically any of the operations that take a string parameter will cause a segfault when operating on an empty stack. A simple test case is:
This should attempt to "execute" an empty string, which on most platforms does nothing, but on FBBI this will generate a segfault because of a null pointer reference.
The fault occurs in the stack_pop_string
function. On an empty stack, the initial stack_pop
call on line 119 returns null, and the while loop is never executed, but since s->sp == -1
is true on an empty stack, the null that was assigned to x
is dereferenced in the final condition on line 126, and thus we get a segfault.
I think the fault can be prevented with the following replacement code:
void stack_pop_string(stack * s, char * str)
{
int i = 0;
void * x = NULL;
SDEBUG("Before Pop String", s);
x = stack_pop(s);
while ((x != NULL) && (*(char *)x != 0))
{
str[i++] = *(char *)x;
x = stack_pop(s);
}
str[i] = (char)'\0';
}
And here's a more thorough test case that validates a couple of edge cases:
"llun ticilpmI ohce"=$ 0"llun ticilpxE ohce"=$ = a"kcats ytpmE"bk, @
In the program above, we're calling the execute command three times - first using a string with an implicit null terminatorr, then with an explicit null terminator, and finally using a completely empty stack. If all the calls are successful, the program should output:
Implicit null
Explicit null
Empty Stack
If we want to get more adversarial, though, there is still another potential segfault. The stack_pop_string
function returns the popped string by copying it into a fixed length buffer (always 256 chars). So we can easily cause a segfault just by using a string longer than 256 characters. A simple test case:
If you want to avoid this fault, you'd need to add a check in the stack_pop_string
function to prevent it overflowing the buffer. Something like this should do the trick:
void stack_pop_string(stack * s, char * str)
{
int i = 0;
void * x = NULL;
SDEBUG("Before Pop String", s);
x = stack_pop(s);
while ((x != NULL) && (*(char *)x != 0))
{
if (i+1 < 256) str[i++] = *(char *)x;
x = stack_pop(s);
}
str[i] = (char)'\0';
}
Although ideally you shouldn't be hardcoding the size like that - it would be safer to pass that in as a parameter from the various calling locations.