Be Careful When Using Scoped Structures In C

2016-04-09 - By Robert Elder

Introduction

I was working my C compiler this afternoon, when I realized the following behaviour of scoped structures that I wanted to share.  If you have the following code, and I guarantee you that 'includes.h' does not have any preprocessor macros in it (only pure C code), is it possible that the following code can ever write past the end of the buffer at position 400?

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

int main(void){
        struct foo {
                struct buffer_object * p;
        };
        struct buffer_object {
                int last_write_position;
        };
        struct foo f;
        int buffer_size = 300;
        int write_attempt_location = 400;
        f.p = malloc(sizeof(struct buffer_object));

        if((f.p->last_write_position = write_attempt_location) < buffer_size){
                printf("Looks good to me!  Wrote to location %d.\n", write_attempt_location);
        }else{
                printf("Woah there!  We're not allowed to write to location %d.\n", write_attempt_location);
        }

        free(f.p);
        return 0;
}

Yes It Is Possible

As you might expect, if 'includes.h' is an empty file, the output will be

Woah there!  We're not allowed to write to location 400.

But, if 'includes.h' contains this structure definition

struct buffer_object{
	char last_write_position;
};

the output will be

Looks good to me!  Wrote to location 400.

Furthermore, this code will not produce any warnings or errors when compiled with reasonable flags as C code regardless of whether the duplicate definition is present or not:

gcc -Wall -pedantic -std=c11 main.c && clang -Wall -pedantic -std=c11 main.c

But it produces errors in c++

g++ -Wall -pedantic main.c || clang++ -Wall -pedantic main.c
main.c: In function ‘int main()’:
main.c:15:43: error: invalid conversion from ‘void*’ to ‘buffer_object*’ [-fpermissive]
  f.p = malloc(sizeof(struct buffer_object));
                                           ^
clang: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
main.c:15:6: error: assigning to 'struct buffer_object *' from incompatible type 'void *'
        f.p = malloc(sizeof(struct buffer_object));
            ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Explanation

Once you realize that the 'struct buffer_object *' pointer can either reference an incomplete type that will be defined later in the current scope, or an already completed type in an outer scope, the problem becomes a bit more obvious.

struct foo {
	struct buffer_object * p;
};

Since the version of the buffer_object structure from includes.h contains an identically named member of a different size, the assignment operation silently overflows the last_write_position variable which is of type 'char'.  The result of the assignment expression is the value assigned, which can never be greater than the maximum value of a char, and in this case it is less than 400 so the check passes.

If anything about this surprised you, it was probably the fact that this will compile with or without the other definition from includes.h.  The other aspect that can lull you into a false sense of security here, is that a quick code review might lead you to conclude that nothing in main.c depends on any global state at all (because it certainly doesn't need to), so as long as includes.h doesn't have any strange macros, you'd probably expect to be safe.

Security Consequences

I thought this example might be interesting from a security point of view in the same way that the underhanded C contest is.  It is likely that everything I've described here is already well known and documented in terms of security best practices.  I tried Googling for it, but it's a hard thing to search for, and I didn't find anything.  An attacker might find this technique interesting for the following reasons:

• You only need to sneak a structure definition into a header dependency of the code you want to attack.
• You can change the behaviour of pre-existing code in a very indirect way without making changes to the code in the file you're targeting.
• This works in pure C, so you wouldn't need to add any suspicious macros.  Hide in plain sight!
• You can leverage a number of subtle and often poorly understood differences related to changing data types in C: usual arithmetic conversion, integral promotion, etc.

The Solution

Haven't you heard?  The solution is to use Rust instead.  All the cool people are using it!

How to Get Fired Using Switch Statements & Statement Expressions Published 2016-10-27	$40.00 CAD C Programming Fridge Magnets	The Jim Roskind C and C++ Grammars Published 2018-02-15	7 Scandalous Weird Old Things About The C Preprocessor Published 2015-09-20
Building A C Compiler Type System - Part 1: The Formidable Declarator Published 2016-07-07	Modelling C Structs And Typedefs At Parse Time Published 2017-03-30	Building A C Compiler Type System - Part 2: A Canonical Type Representation Published 2016-07-21	The Magical World of Structs, Typedefs, and Scoping Published 2016-05-09