3

I have the following code:

_Bool grantAccess(char *password){
    char goodPassWord[]= "goodpass";
    return (0 == strcmp(password, goodPassWord));

}

_Bool grantAccessExercise(void){
    char password[9];
    int allow = 0;

    printf("Please enter password: ");

    gets(password); 

    if (grantAccess(password)) {
         allow = 1;
    }

    return (allow != 0);
    }

When I enter any combination of 10 characters for password it overflows and overwrites the null-terminator. Can anyone explain why the non null-terminated value causes StrCmp to return 0?

qz_99
  • 185
  • 1
  • 12
  • 2
    It's undefined behavior, so there's no guaranteed result of `0`. It might as well do something else entirely. – Blaze Nov 23 '18 at 12:45
  • Because of [*undefined behavior*](https://en.wikipedia.org/wiki/Undefined_behavior). – Some programmer dude Nov 23 '18 at 12:45
  • Please read https://faq.cprogramming.com/cgi-bin/smartfaq.cgi?answer=1049157810&id=1043284351 and why it is bad – Ed Heal Nov 23 '18 at 12:47
  • 5
    And note that the [dangerous](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used) function `gets` doesn't "overwrites the null-terminator", rather it writes the null-terminator *out of bounds* of your array. – Some programmer dude Nov 23 '18 at 12:47
  • The undefined behavior consistently returns 0 which would suggest defined behavior though – qz_99 Nov 23 '18 at 12:50
  • 3
    Just because something seems to work doesn't mean the behavior actually is *defined*, just that you happen to be "lucky" (some would consider it ***un**lucky*). You don't know when it might break, or what you did to make it break once it happens. – Some programmer dude Nov 23 '18 at 12:52
  • I understand. I was thinking there might be some kind of weakness in StrCmp that caused it to always return a positive result when passed NNT values. I guess my lecturer can't argue with 'undefined behavior'. Thanks everyone. – qz_99 Nov 23 '18 at 13:04
  • "and overwrites the null-terminator" There is no nul-terminator before you call `gets`, therefore it cannot overwrite it. And there will be a terminator after the call. Just not withing the array limits. – Gerhardh Nov 23 '18 at 13:16
  • 1
    @Hughes_J Be aware that you need to differentiate between *'implementation defined behaviour'*, meaning that the behaviour is not defined by the standard, but left to compiler vendors (which then need to document how their respective implementations behave) and *'undefined behaviour'* which means that there doesn't exist any correct behaviour and what ever you do, the result cannot be correct (such as calculating real square root from negative real values). – Aconcagua Nov 23 '18 at 13:18

2 Answers2

4

Can anyone explain why the non null-terminated value causes StrCmp to return 0?

This is not what happens.

What happens is:

  • the buffer overflow over password overwrites bytes that are part of the stack-located variable allow
  • as a result, allow does no longer contain the value zero, but some other value.
  • the call to grantAccess() returns false, and allow is not modified.
  • at the end, allow contains the non-zero value due to the overflow.

In order to verify that, I made a test as follows:

  • I entered password "0123456789"
  • I observed that allow == 57, which is the ASCII code of character '9'.
user803422
  • 2,636
  • 2
  • 18
  • 36
  • 1
    In other words, TL;DR, strcmp does **not** return 0. – hyde Nov 23 '18 at 13:19
  • 1
    Note that this still is UB. There is no guarantee to the ordering of values (variables) on the stack. But +1 for spotting the reason for the seemingly strange behavior of strcmp in this example. – GermanNerd Nov 23 '18 at 13:20
0

Further clarification: What you observe here is stack corruption. If you input a long enough string as your password, you'll even get stack smashing. You can play around with giving your password array different sizes, which might lead to a reordering of things on the stack. A good explanation of stack smashing or stack buffer overflow is here.

You can also change your code to allocate the password array on the heap using malloc. You might get seemingly working code, as most memory locations, when incremented, will at some time very likely contain a 0, which is interpreted as the NULL terminator. This would be a much more insidious behaviour, since your grantAccess function might seem to work correctly.

GermanNerd
  • 643
  • 5
  • 12