Thread: C Buffer Overflow Problem

Hey,
I am having a problem with a very simple code I have written to test buffer overflows in C. There are two versions of my code, a version that should be (and is) vulnerable to buffer overflows and one that should not be vulnerable.
This is the vulnerable version:

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>


int check(char *password) {
    
    int auth_flag = 0;
    char password_buffer[16];
    
    strcpy(password_buffer, password);
    
    
    if(strcmp(password_buffer, "password") == 0)
        auth_flag = 1;
    if(strcmp(password_buffer, "testest") == 0)
        auth_flag = 1;
    
    return auth_flag;
    
}

int main(int argc, char *argv[]) {
    if(argc < 2) {
        printf("Syntax: %s <password>\n", argv[0]);
        exit(0);
    }
    if(check(argv[1])) {
        printf("Access Granted.\n");
    } else {
        printf("\nAccess has been Denied.\n");
    }
}

The program actually works, when I enter the correct passwords it prints "Access Granted" like it is supposed to do and when I enter a long string, my password_buffer overflows into the auth_flag, changing its value and also granting access.
Then I tried to create a version without that vulnerability by switching my variables like that:

Code:

int check(char *password) {
    
    char password_buffer[16];
    int auth_flag = 0;
    
    strcpy(password_buffer, password);
    (...)

My idea was, that when I switch the variables in my code, auth_flag should be located in memory before password_buffer, so it could not be overwritten.
And that is my problem: it does not work. I can still overflow into the flag and when I look into the assembler code, nothing really changes. I know that I could fix this problem by simply making both variables global, but I have already seen in someone else's code, who did the same thing to prevent overflows, that it should work the way I tried it.
Has anybody an idea what is wrong?

The compiler isn't obligated to order variables in any particular way. It's possible that it orders them the same way in both versions. In fact, that's what it does for me (Ubuntu 14.04, gcc 4.8.4), putting auth_flag before password_buffer so I can't overflow into it in either case. Try putting the following before the strcpy:

Code:

    printf("%p %p\n", (void*)&password_buffer, (void*)&auth_flag);

An obvious way to avoid overflow in this case is:

Code:

int check(char *password) {
    return strcmp(password, "password") == 0
        || strcmp(password, "testest") == 0;
}

(But you probably know that!)