CODEWITHSUNDEEP
securitystackbuffer-overflow
Sebastian M
Sebastian M

Reputation: 491

Why is this code vulnerable to buffer overflow?

void cpy(char* b) {
    char values[1024];
    strcpy(b, values);
    fprint(values);
}

int main(int argc, char** argv){
    if(argc == 1 || strlen(argv[1]) > 1024) {
        fprint("Nope!\n");
        return 1;
}
    cpy(argv[1]);
    return 0;
}

Why is this code vulnerable to buffer overflow? I think it has something to do with the "strcpy" part but I'm not sure...

Any ideas?

Upvotes: 0

Views: 249

Answers (3)

Jonas Schäfer
Jonas Schäfer

Reputation: 20738

Short story: The arguments on strcpy are switched.

Long story: strcpy copies from the second to the first argument.

Let us make a quick analysis. In main, the code checks that argv[1] is at most 1023+1 (NUL byte) characters long. argv[1] is then passed to cpy as first and only argument and is available there as b.

In cpy, there’s also an uninitialised array of char called values, which is allocated to be 1024 characters long.

Now, strcpy is instructed to copy from values into b. As we know, b is the pointer obtained from argv[1], and thus has at most 1024 chars of space. values is reserved for 1024 characters, but uninitialised. Thus, it may or may not contain a NUL byte in those 1024 characters.

If it happens to contain a NUL byte before the bounds of argv[1] are reached, everything is fine. If it does not, two things can happen:

  • if argv[1] is exactly 1023 characters long (+ terminating NUL byte), out of bounds reads (on values) and writes (on argv[1]) will happen.

  • if argv[1] is less than 1023 characters long, out of bounds writes on argv[1] will happen, and by extension there may also happen out of bounds reads on values, if the program survives the out of bounds writes.

Depending on what fprint is (I don’t have a manpage for it on my system), there may be other issues in the code.

Upvotes: 2

Michael Möbius
Michael Möbius

Reputation: 1050

The problem is the fprint, it does not know when your string ends. It should end with a '\0'. So if you write something in your array with all the 1024 bytes set. The fprint command will read out of the memory to check if there is a '\0'... It will continue to read until it finds the end mark.

Upvotes: 0

r4dic4c3d
r4dic4c3d

Reputation: 1

I think you use a array of char as there is no operator to mark the end of your array.

Upvotes: 0

Related Questions