Re: Format String Attacks

[Ajax <ajax () FIREST0RM ORG>]

Forgive my naivete here, but...  Isn't the essential problem, besides poor
coding practice which we all know is not an excuse yadda yadda yadda, the
fact that C doesn't define a way to detect the end of a va_list?  We have
va_start(), va_arg(), and the misleadingly named va_end(), but no
"va_last()".

Would it be that hard to implement an int va_last(va_list ap) that would
return true if ap pointed to the last element in a set of varargs?

The default stdarg.h implementation for gcc seems to do this (for
little-endian architectures, comments mine):

/* init AP to the next arg we pop from the stack */
#define va_start(AP, LASTARG)                                           \
 (AP = ((__gnuc_va_list) __builtin_next_arg (LASTARG)))

/* advance the AP pointer and return the next arg */
#define va_arg(AP, TYPE)                                                \
 (AP = (__gnuc_va_list) ((char *) (AP) + __va_rounded_size (TYPE)),     \
  *((TYPE *) (void *) ((char *) (AP) - __va_rounded_size (TYPE))))

Note how this works; AP is treated as, essentially, void *AP[], an array
of pointers to arbitrary types.  This creates a natural terminating
condition, where the last element in the array is NULL (_not_ a pointer to
NULL).

Now, we can cheat.  A va_list is just a void * {1}, so we could build our
own void *array[] somewhere, and do like
    vfprintf(stdout, string, (va_list)array);
This is the programmer intentionally shooting herself in the foot, so I
don't much care.  I could do
    void *p = malloc(random()); free(p); free(p);
too, which would also cause a DoS.

Regardless, functions should be smarter than their users, we already check
for invalid arguments all over the place, why not harden vararg functions
too.

So is this notional va_last() implementable, without breaking any ABIs?

I'm not sure.  I believe so.  It looks like, contrary to my initial
suspicion, most architectures pass varargs in a similar fashion (a pointer
to an array of (possibly struct-enclosed) pointers, as the last argument
to a function).  The Alpha, the Clipper, the i860, the 80960, the m88k,
(sometimes) the MIPS, the PowerPC, the Pyramid compiler and the SH3 ABIs
appear to define va_list as a struct rather than a simple type, but unless
I'm missing something obvious, and discounting the programmer cheating, I
can't think of a situation where the compiler could *not* know the number
of args in a va_list.

So, how va_last()?  Say we have
    int my_printf_wrapper(char *format, ...) {
        va_list ap;
        va_start(ap, format);
        vprintf(format, ap);
        va_end(ap);
    }

When the compiler constructs the va_list that ap will point to, it simply
makes the last element NULL, which can (ought) never be a valid pointer to
anything.  ap then looks like:

ap + --> &a             ap + --> &a
   |                       |
   + --> &b      NOT       + --> &b     (we can still pass NULL
   |                       |            pointers as varargs)
   + --> &c                + --> &c
   |                       |
   NULL                    + --> NULL

va_last becomes a simple check for NULL.  Suppose we define va_last()
    int va_last(va_list ap);

printf()-like functions can make (va_last(ap)) a terminating condition for
their argument-parsing loop, and bomb out with EINVAL if it hits the last
argument before it hits the last format string.

Am I wrong?  Does this break any ABI?  (I can't imagine tacking a NULL
onto the end of a variable-length list would produce invalid behavior, but
hey, some people think *((void *)0) == 0.)  Granted it's a non-ANSI
extension.  So what.  So's mkstemp(3), but we all use it (right?).

At the very least I'd imagine a "proactively secure" OS would see this as
a valid paranoia check.

Feedback good.

{1} - Not entirely true.  Some architectures, like the m88k, pass the
first n arguments in registers and the rest in a list or stack somewhere.
For those, the va_list is a struct that specifies the border somehow.  It
might be a little troublesome to do this on arches that pass args on-stack
without breaking an ABI.  I think.  This is why I'm asking for feedback.

-=:[ ajax (firest0rm)