Stack smashing detected with fgets() function in C

Question

I got this code:

while(!feof(fp))
{
   fgets(line,20,fp);
   puts(line);
}

fp is this file:

prova1
prova2
prova3prova4
prova5

The fact is: if the count parameter of the fgets function it's a lower number than 10 it works everything ok, but if it's greater it gives me a stack smashing detected error.

I think it's because I'm trying to read after EOF, but why when I'm trying to read just after it works (like 8 characters per line) but when I put a count parameter of like 20 it gives me an error?

NB: the last line "prova5" is formed by 6 characters plus the '\0' I think, so 7. Why with 8 it's ok and it's not with 20?

EDIT: I'm sorry, I forgot to include this declaration:

char line[] = " ";

A good ideom is to use `fgets(line, sizeof(line), fp);` if `line` is an array (does not work if it is a pointer). Then it cannot write out of bounds because the specified size does not correspond to the array size. — Ctx, Dec 11 '19 at 13:55
Please read [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) — Some programmer dude, Dec 11 '19 at 13:56
Lastly, C doesn't have any kind of bounds checking. Going out of bounds leads to *undefined behavior*, so just don't do it. — Some programmer dude, Dec 11 '19 at 13:58
If you see "stack smashing detected" this usually means you write beyond the end of a locally declared array. — Jabberwocky, Dec 11 '19 at 14:09
@ryyker: Well, that certainly removes any question that might have lingered about buffer overflow, doesn't it? — Fred Larson, Dec 11 '19 at 19:57

ryyker · Accepted Answer · 2019-12-11T19:53:14.483

Why with 8 it's ok and it's not with 20?

This suggests the possibility that line has not been defined with enough space for containing 20 char. To be in line with your code snippet, line should be defined as:

char line[20];

Depending on your actual definition of line, you could be seeing results of buffer overflow. The reason it seems to work, then not work is undefined behavior, which means any behavior is possible, and at unexpected times.

Also, you should consider replacing while(!feof(fp)) with while(fgets(line, 20, fp)) which checks the return of fgets as a test, and exits upon seeing NULL. feof() (is broken)

while(fgets(line, 20, fp))//will exit loop upon a null return
{
   //do something with line;
   puts(line);
}

EDIT to address clarification in comment:

If you declared line as a pointer:

char *line;

Then before using it in the rest of your code the pointer needs to be given an address with corresponding memory sufficient to contain 20 char. This statement (and test to ensure memory was allocated) will do both:

line = calloc(20, 1);
if(line)
{
    //use line as described above

EDIT 2 to address new information in OP.

Since line was defined as (edited into OP later)

char line[] = " ";

It has room for only 1 character. This will cause buffer overflow, and explains the inconsistent behavior that was observed (and written about in the post.)

The suggested way to define line to support what the code shown in your post needs is:

char line[20] = {0}; //space for 20 char, initialized to all zeros.

This, along with the other suggestions about replacing feof with fgets will improve the performance you see, and eliminate undefined behavior due to buffer overflow

I defined line as a pointer, I forgot to include the declaration in the question — Pierpaolo Vesce, Dec 11 '19 at 16:00
@PierpaoloVesce - If you include that declaration in your original post, it will remove the reason your question was closed, and I will vote to re-open. In the mean time, I will edit my answer to address how you can use the pointer with `malloc` to make your code work. By the way, if you did not allocate memory before using your pointer, that would explain the undefined behavior that people (including me) were citing. — ryyker, Dec 11 '19 at 16:04
I would have defined line as a pointer but I changed into an array and yes if you can tell me how to use the pointer with malloc I would be so grateful, I'm so used to C++ that programming in C seems terrible — Pierpaolo Vesce, Dec 11 '19 at 16:06
@PierpaoloVesce - Have updated my answer, and voted to reopen your post. Thank you for updating. — ryyker, Dec 11 '19 at 18:06

Stack smashing detected with fgets() function in C

1 Answers1