Is this buffer overflow?

Question

Really confused if my program is behaving the way it's supposed to. this isn't homework, just a fun march madness predictor program I'm writing.

    char buffer[20];
    char team1_name[20];    // 18 chars + new line + null
    char team2_name[20];

    printf("Who is team 1?\n");
    fgets(buffer, sizeof(buffer), stdin);
    strncpy(team1_name, buffer, sizeof(team1_name));
    team1_name[strlen(team1_name) - 1] = '\0';

    printf("\nWho is team 2?\n");
    fgets(buffer, sizeof(buffer), stdin);
    strncpy(team2_name, buffer, sizeof(team2_name));
    team2_name[strlen(team2_name) - 1] = '\0';

    printf("\nEnter %s's info:\n", team1_name);

Out of curiosity I enter a team name greater than 20 characters and it completely skips over the second print statement. am I protecting against buffer overflow? is it up to the user to not put in something huge? Do I need flush statements?

This is the output:

    Who is team 1?
    wjefowiejfwoiejfweoifjweoifjweofijweoifj

    Who is team 2?

    Enter wjefowiejfwoiejfwe's info:
    Wins in last 12:

    Losses in last 12:

    Points per game:

Your `fgets` lines only read as many characters as you tell them to. If the user enters more than that, they stay for the next `fgets`. — David Schwartz, Mar 13 '12 at 07:45
@Tim: just keep reading until you get the newline, and then ask your next question. This isn't rocket science, it's basic input handling. — tbert, Mar 13 '12 at 07:55
The simplest solution is to read lines into a larger buffer, say 128 bytes. That will ensure you consume an entire line unless the user is really crazy. You can also read character-by-character until you read a line ending, but that seems kind of like overkill. — David Schwartz, Mar 13 '12 at 07:56

score 1 · Accepted Answer · answered Mar 13 '12 at 07:48

1

The problem is that, since your input is truncated in the first fgets (you have more than 20 chars), then the second fgets will read the end of the FIRST input string from stdin.

Display "team2_name" value to see it (it contains chars after the 20 first chars in team1_name).

Oops, sorry, the following comment was wrong. Forget about it : And yes, fgets MUST use sizeof(buffer)-1, because this argument is the max number of chars read. So if you read sizeof(buffer) chars, you will need sizeof(buffer)+1 chars to store them (including trailing '\0')

answered Mar 13 '12 at 07:48

huelbois

6,762
1
19
21

ok that makes sense. is there anyway in C to prevent this fgets overflow? something that just ignores the extra characters and would let the user enter team 2's name anyways? – Tim Mar 13 '12 at 07:56
1

I think you will have to write a function for yourself. No a complicated task, you just have to check that you got the '\n' in the returned string. If it's not the case, you have to keep on calling fgets (into another buffer) until you find the '\n'. (fgets stops when it has read the count-1 of chars OR a \n, whichever comes first). – huelbois Mar 13 '12 at 08:05
ok i finally got it. i didn't understand that fgets needs the newline. i was thinking printf with \n took care of it, fflush wasn't doing anything either. mocking me saying it wasn't rocket science didn't help either. – Tim Mar 13 '12 at 08:11

score 1 · Answer 2 · answered Mar 13 '12 at 07:49

You read 20 chars out of stdin, but the rest of the input still remains in the stream. The next fgets reads those remaining chars, so Team1 is called wjefowiejfwoiejfwe and Team2 oifjweofijweoifj. Print Team2's name as well and you will see this is true.

Is this buffer overflow?

2 Answers2