I am using scanf()
to get the password from the user and store it in password
variable which has a size of 20 bytes. I check the entered password against the the correctPassword
and if they match, boolean variable pass
will be changed to true
.
So, when I enter a password which is longer than 20 chars, a buffer overflow happens and the value of pass
becomes non-zero (ie true). However, when I use printf()
to print the address of variable pass
no buffer overflow happens even-though I use a password which is longer than 20 chars.
here is the code that leads to overflow:
#include <stdlib.h>
#include <stdio.h>
#include <stdbool.h>
#include <string.h>
int main (int argc, char *argv[]) {
char password[20];
char correctPassword[] = "random";
bool pass = false;
printf("enter your password: ");
scanf("%s", password);
if (strcmp(password, correctPassword) == 0) {
// compare the two strings,strcmp() returns 0 if two strings values are the same.
pass = true;
}
if (pass) {
printf("Connecting you to the central system...\n");
} else {
printf("Password is wrong! entry denied\n");
}
printf("%d\n", pass);
return 0;
}
when password entered is in this case 40 chars long(ASCII value of a
is 97), value of pass changes to 97(true).
enter your password: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
Connecting you to the central system...
97
here is the same code but with one extra line at the end to print the address of variable pass
:
#include <stdlib.h>
#include <stdio.h>
#include <stdbool.h>
#include <string.h>
int main (int argc, char *argv[]) {
char password[20];
char correctPassword[] = "random";
bool pass = false;
printf("enter your password: ");
scanf("%s", password);
if (strcmp(password, correctPassword) == 0) {
// compare the two strings,strcmp() returns 0 if two strings values are the same.
pass = true;
}
if (pass) {
printf("Connecting you to the central system...\n");
} else {
printf("Password is wrong! entry denied\n");
}
printf("%d\n", pass);
printf("%x\n", &pass);
return 0;
}
In this case password entered is 40 chars long but pass
is still 0
(false).
enter your password: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
Password is wrong! entry denied
0
61fec4
So, my question is how could adding only
printf("%x\n", &pass);
leads to no buffer-overflow as expected?
scanf("%s", password)
is a security flaw: any input word longer than 19 bytes will cause scanf()
to write beyond the end of the array, triggering undefined behavior.
Undefined behavior can have unpredictable side effects, which can be visible or not. The output of 97
is a mild side effect, but a long enough input will corrupt the return address and cause a segmentation fault upon returning from main()
and a cleverly constructed input might allow an attacker to execute arbitrary code. Changing the program likely changes the side effects as the allocation of the local variables may be different for example when you take the address of &pass
. None of this is predictable.
Note also that printf("%x\n", &pass);
has undefined behavior too because %x
expects an unsigned int
, not a bool *
, you should write printf("%p\n", (void *)&pass);
There is a simple way to prevent this by passing a length field to scanf()
:
scanf("%19s", password);
Note however that you should also check the return value to detect premature end of file and you should also flush the rest of the input line.
Here is a modified version:
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main (int argc, char *argv[]) {
char password[20];
char correctPassword[] = "random";
bool pass = false;
int c;
printf("enter your password: ");
if (scanf("%19s", password) != 1) {
printf("invalid input\n");
return 1;
}
/* read and discard the rest of the line */
while ((c = getchar()) != EOF && c != '\n')
continue;
if (strcmp(password, correctPassword) == 0) {
// compare the two strings,strcmp() returns 0 if two strings values are the same.
pass = true;
}
if (pass) {
printf("Connecting you to the central system...\n");
} else {
printf("Password is wrong! entry denied\n");
}
printf("%d\n", pass);
return 0;
}