Neverflow: C macros that guard against buffer overflows (opens in new tab)

The problem with C and buffer overflows isn't that you can't guard against them, or that there is no existing, reusable code to do so — it's that none of this functionality is standardized. Adding another one to the existing 41383 ways of doing this is in fact the exact opposite of what's needed. Ideally C needs one way of doing this, and that would be described in the standard.

But that's not how C "rolls", and we'll never get that. So I guess we now have 41384 ways to do buffer overflow guards.

Even without array bounds checking, a bit of discipline and smart conventions will go a long way of reducing errors:

1. Define a macro function for retrieving the length of an array:

  #define LEN(arr) (sizeof (arr) / sizeof (arr)[0])

2. Don't introduce macro constants for array lengths; hard code the length in the declaration and use LEN to retrieve it. Example:

  int a[100];
  ...
  for (i = 0; i < LEN(a); i++) {
     ...
  }

3. Define a macro function for dynamic array allocation:

  #define NEW_ARRAY(ptr, n) \
     (ptr) = malloc((n) * sizeof (ptr)[0]); \
     if ((ptr) == NULL) { \
        fprintf(stderr, "Memory allocation failed: %s\n", strerror(errno)); \
        exit(EXIT_FAILURE); \
     }

4. When you create a function with an array argument, also add an argument for the array length.

5. Use a convention for naming the length of array pointer targets, for instance by adding the suffix `Len'. Example:

  int *b, bLen = 100;
  ...
  NEW_ARRAY(b, bLen);  /* nice to know that b and bLen belong together */
  ...
  SomeFunction(b, bLen, ...);
  ...
  for (i = 0; i < bLen; i++) {
    ...
  }
 

6. Define your own safe wrappers around unsafe standard library functions or use someone else's code that does that.

C23 improved struct compatibility so you might be able to leverage that to craft macros that better emulate slices. [1]

There is an RFC proposal for the Clang frontend for adding bounds checking reminiscent of Microsoft's SAL. [2]

[1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3003.pdf

[2] https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-...

The following error prone: it can be mistakenly applied to a pointer:

#define LEN(NAME) (sizeof NAME / sizeof(NAME)[0])

I think gcc has a warning for this pattern now: when the size of a pointer is divided by the size of its referent type.

More importantly, it has an odd extra level of indirection. The traditional definition is:

#define LEN(ARRAY) (sizeof ARRAY / sizeof (ARRAY)[0])

This means that to use LEN on an array, we have to take the address:

   char *array[5];
   LEN(&array);  // -> 5

If we use

   LEN(array);

which is an easy mistake, we get:

    sizeof *array / sizeof (*array)[0]

which is

    sizeof (char *) / sizeof (char)

which is

    sizeof (char *)

which is likely 4 or 8.

I do see that LEN is supposed to be (only) used in conjunction with ARR:

    #define ARR(TYPE, NAME, COUNT) TYPE(*NAME)[COUNT]

but that isn't enforced. An idea would be to add some "secret" prefix or suffix to NAME like blah_ ## NAME, so that name cannot be referenced without going through the macros; i.e. if we define ARR(int, foo, 42) then there is no declared identifier foo; it actually declares blah_foo, and LEN(foo) knows about that, also adding the prefix. Thus mistakenly using LEN(foo) on something not declared with ARR will likely be a reference to an undeclared identifier.

The expansion of the AT macro seems a bit bloated:

  #define AT(NAME, IDX)                                         \
    ((typeof(&(*NAME)[0]))                                      \
    ((ASSERT(((size_t)IDX) * sizeof(*NAME)[0] < sizeof *NAME,   \
    "Buffer Overflow. Index [%lu] is out of range [0-%lu]",     \
    ((size_t)IDX), ((sizeof *NAME / sizeof(*NAME)[0]) - 1))),   \
    ((uchar *)*NAME) + ((size_t)IDX) * sizeof(*NAME)[0]))

Some of this might be pushed into non-inlined run-time support function. That could be static and defined in the header, to keep it header-only, but ideally there would be a .c file so it's defined only once.

When you factor in the definition of ASSERT, and the ERRLOG macro that is using, it's a lot of cruft for just one array access.

Some compile-time options (via preprocessor macros) to control the bloat would be useful; e.g. a way of compiling it so that AT will just predictably crash, without a detailed error message with __FILE__ and __LINE__ and all. Basically just the check, with a branch to some code that calls abort() if it's out of bounds.

Interesting idea, although given the demotion into optional feature in C11, it isn't necessarly portable.

Also doesn't cover all the string and memory buffer manipulations.

SAL and Frama-C are the bare minimum for security in C code.

Why use C and keep reinventing things that C++ provides?

Related ongoing thread:

Modern C (2019) - https://news.ycombinator.com/item?id=36167820 - June 2023 (19 comments)

Runtime bounds check tied to fprintf and abort via macros. Allocation by calloc.

This evaluates macro parameters multiple times, so if the parameters have side effects or evaluate inconsistently this won't work. For example:

    size_t SomeIndex() {
      static size_t example_index = 0;
      return example_index++ % 2;
    }

    int main() {
      NEW(int, arr, 1);
      // This buffer overflow is not detected:
      *AT(arr, SomeIndex()) = 42;
      return 0;
    }

Never heard of a serious buffer overflow caused by _constant_ indices. Does it work with AT(arr, i), or only with AT(arr, 10)?

See also here for my experiments, but it relies on UBSan for bounds checking: https://github.com/uecker/noplate.git

The best way to deal with this kind of thing is to write a small language that transpiles to the subset of c that you are using.

Here is a different take on it. We can use #define to inform the header about the properties of certain symbols.

Here is my oob.c program. I will show the output, and then the content of "oob.h".

  #include <stdlib.h>
  #include <stdio.h>
  #include "oob.h"

  int oob_fail(const char *file, int line)
  {
    fprintf(stderr, "%s:%d:out of bounds array access\n", file, line);
    abort();
  }

  /*
   * Declare properties of array type x
   */
  #define ARRAY_ELTYPE_x int    /* element type is int */
  #define ARRAY_SIZE_x 7        /* number of elements is 7 */

  /*
   * Ensure array type x is fully declared at file scope
   */
  ARRAY_FULLTYPE(x);

  /*
   * Inform the OOB module that the identifiers p and a are
   * used as variables related to type x: either pointers
   * to it or values.
   */
  #define ARRAY_TYPEOF_p x
  #define ARRAY_TYPEOF_a x

  int get_elem(ARRAY_TYPE(x) *p, int i)
  {
     return APREF(p, i);
  }

  int main(void)
  {
     ARRAY_TYPE(x) a = ARRAY_INIT(1, 2, 3);

     for (size_t i = 0; i <= ARRAY_SIZEOF(a); i++)
        printf("a[%zd] == %d\n", i, get_elem(&a, i));

     return 0;
  }

Output:

  $ ./oob
  a[0] == 1
  a[1] == 2
  a[2] == 3
  a[3] == 0
  a[4] == 0
  a[5] == 0
  a[6] == 0
  oob.c:31:out of bounds array access
  Aborted (core dumped)

The content of "oob.h"

  #ifndef OOB_H_435E_FDE9
  #define OOB_H_435E_FDE9

  int oob_fail(const char *file, int line);

  #define OOB_PREFIX oob_ident_
  #define OOB_XCAT(X, Y) X ## Y
  #define OOB_CAT(X, Y) OOB_XCAT(X, Y)

  #define ARRAY_ELTYPE(T) OOB_CAT(ARRAY_ELTYPE_, T)
  #define ARRAY_SIZE(T) OOB_CAT(ARRAY_SIZE_, T)
  #define ARRAY_TAG(T) OOB_CAT(ARRAY_TAG_, T)

  #define ARRAY_FULLTYPE(T)                                                     \
    struct ARRAY_TAG(T) {                                                       \
      ARRAY_ELTYPE(T) a[ARRAY_SIZE(T)];                                         \
    }

  #define ARRAY_TYPE(T) struct ARRAY_TAG(T)

  #define ARRAY_TYPEOF(V) OOB_CAT(ARRAY_TYPEOF_, V)
  #define ARRAY_SIZEOF(V) ARRAY_SIZE(ARRAY_TYPEOF(V))

  #define ARRAY_INIT(...) { { __VA_ARGS__ } }

  #define AREF(ARRAY, I)                                                        \
    (((size_t) (I) >= ARRAY_SIZEOF(ARRAY))                                      \
     ? oob_fail(__FILE__, __LINE__), (ARRAY).a[0]                               \
     : (ARRAY).a[I])

  #define APREF(PARRAY, I)                                                      \
    (((size_t) (I) >= ARRAY_SIZEOF(PARRAY))                                     \
     ? oob_fail(__FILE__, __LINE__), (PARRAY)->a[0]                             \
     : (PARRAY)->a[I])

  #endif

Preprocessor invoked on oob.c (snipped down to the relevant part after the run-time support function oob_fail):

  struct ARRAY_TAG_x { int a[7]; };


  int get_elem(struct ARRAY_TAG_x *p, int i)
  {
     return (((size_t) (i) >= 7) ? oob_fail("oob.c", 31), (p)->a[0] : (p)->a[i]);
  }

  int main(void)
  {
     struct ARRAY_TAG_x a = { { 1, 2, 3 } };

     for (size_t i = 0; i <= 7; i++)
        printf("a[%zd] == %d\n", i, get_elem(&a, i));

     return 0;
  }

It's clean enough to be readable (except, of course, code dense with AREF or APREF calls will be a mess). Uses arrays wrapped in structs, so you can pass arrays by value.

You have to make a list of your variables that are involved and write some #define lines for them.

Same for the array types.