Return to Revisions

Use-after-free bug:

In stack_pop():

    --s->size;
    void *const top = (char *) s->data + (s->size * s->memb_size);
        
    if (s->size && (s->size <= s->cap / 4)) {
        void *const tmp = realloc(s->data, s->cap / 2 * s->memb_size);
        
        if (tmp) {
            s->data = tmp;
            s->cap /= 2;
        } 
        /* Else do nothing. The original memory is left intact. */
    }

The lifetime of s->data ends with the call to realloc(), assuming it succeeded, so top is pointing to a block of memory that might have already been freed whilst shrinking.

GCC 12.3 caught the bug with -O1 -Wall -Werror -Wpedantic.

In file included from <source>:269:
<source>: In function 'main':
<source>:291:16: error: pointer used after 'realloc' [-Werror=use-after-free]
  291 |         assert(*(size_t *) gstack_pop(stack) == i);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'stack_pop',
    inlined from 'main' at <source>:291:9:
<source>:238:27: note: call to 'realloc' here
  238 |         void *const tmp = realloc(s->data, s->memb_size * new_cap);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

If you remove -O1 it doesn't detect anything. GCC 13.1 and 13.2 did not detect the bug, or if they did, they did not output anything.

Valgrind also reports a read error for this in stack_pop().

The solution is to move the statement to the end of the function:

return (char *) s->data + (s->size * s->memb_size);

This also eliminated the variable top.

Incomplete documentation:

The documentation should include a warning about pushing function pointers onto the stack, as function pointers may not be compatible with a void *.

Aside:

stack_is_empty(), stack_is_full(), stack_size(), and stack_peek() are pure functions and may benefit from __attribute__((pure)). They are not constant functions because they're accessing global memory.