#include <stdio.h>
#include <string.h>
char username[64], password[64], hostname[64];
int main(int argc, char **argv) {
char result[256];
if (argc != 4 ||
strlen(argv[1]) > sizeof(username) ||
strlen(argv[2]) > sizeof(password) ||
strlen(argv[3]) > sizeof(hostname)) {
fprintf(stderr, "bad arguments\n");
return -1;
}
strcpy(username, argv[1]);
strcpy(password, argv[2]);
strcpy(hostname, argv[3]);
result[0] = 0;
strcat(result, "http://");
strcat(result, username);
strcat(result, ":");
strcat(result, password);
strcat(result, "@");
strcat(result, hostname);
strcat(result, "/");
printf("%s\n", result);
return 0;
}
1 Answer
Note that your size comparisons are off-by-one.
strlen(argv[1]) > sizeof(username) ||
strlen(argv[2]) > sizeof(password) ||
strlen(argv[3]) > sizeof(hostname)
In the event a string length is 64, there won't be room for the null-terminating byte, thus overflowing the buffer(s).
The fix is the >= operator.
As an aside: the copies are meaningless; lengths can be asserted, and the contents of argv used directly instead.
strlenon non-sanitized input is always wrong. The size checks might be able to prevent buffer overflow hacks, but they won't prevent someone from crashing the program. Instead of usingstrlenyou should do something likememchron the 64 first bytes of each argument to see if there is a null terminator present.