gets() is dead

[Richard Heathfield]

CBFalconer said:

Which is all one more reason to use ggets.

Chuck, this subthread started with Mr Brennan's own attempt at a dynamic
string capture routine, which - whilst not perfect - was a fair stab at
the problem, and one which he was clearly prepared to improve. So why
would he want to use ggets?

The call is simpler
(ggets(&buf) and the results always have the terminal \n
removed. You still have complete error reporting. ggets (and
fggets) are completely portable.

And still subject to memory exhaustion attacks.

 

[Al Balmer]

I'd be happy around here to be considered competent. On the other hand,
for my "skills assessment" at work, I blithely checked "expert" level
for C. I know my competition.

Well put Same here.

 

[Richard Heathfield]

Harald van D?k said:

I don't. Are you perhaps assuming that the separate processes are
separate programs? If so, yes, running the intended child separately
would be unsafe, but there is no need to make separate programs
available. The same program can act both as parent and as child.

If I understand you correctly, you're suggesting that a program could
feed its own output to itself by stuffing it down a pipe and picking it
up with gets(). This seems ludicrously pointless.

 

[¬a\\/b]

something like:

unsigned h;
char buf[2048];
if((h=fgets(buf, sizeof buf, stdin))==0 || buf[h-1]!='\n')
error();

unsigned h;
char buf[2048];
if((h=fgets(buf, sizeof buf, stdin))<=1 || buf[h-2]!='\n')
error();

"" error
"\n" ok
"fifjfj\n" ok
"jfofif^D" error
"jfofif^Z" error
2070 string error
etc

or something like:
char buf[2048], *h;
if((h=fgets(buf, sizeof buf, stdin))==0 || *h!='\n')
error();

char buf[2048], *h;
if( (h=fgets(buf, sizeof buf, stdin))==buf || h[-1]!='\n')
error();
fgets return buf on input error
in the ok case return the h in buffer that *h=0

i can not remember all (nor use them much in the direct way) but i
think they are like as the last i wrote

 

[Walter Roberson]

Harald van D?k said:

If I understand you correctly, you're suggesting that a program could
feed its own output to itself by stuffing it down a pipe and picking it
up with gets(). This seems ludicrously pointless.

I believe he is talking about fork(), in which case you -do- have
control over the input and output because both sides are the same
program (though not the same process.)

Forking and communicating between the processes with a pipe is
common for cases where a portion of the work must be done with
different access permissions.

 

[Flash Gordon]

¬a\/b wrote, On 27/04/07 08:56:

fgets should to return the number of char successfull written in
buffer;

<snip more stuff based on this completely false statement>

Wrong. fgets is defined by the C standard as returning a null pointer on
failure or a pointer to the start of the buffer on success. I suggest
you go back to school.

 

[Flash Gordon]

¬a\/b wrote, On 27/04/07 19:45:

something like:

unsigned h;
char buf[2048];
if((h=fgets(buf, sizeof buf, stdin))==0 || buf[h-1]!='\n')
error();

unsigned h;
char buf[2048];
if((h=fgets(buf, sizeof buf, stdin))<=1 || buf[h-2]!='\n')
error();

Still based on a completely incorrect belief of how fgets is defined.

or something like:
char buf[2048], *h;
if((h=fgets(buf, sizeof buf, stdin))==0 || *h!='\n')
error();

I failed to spot this before. How can you possibly believe that fgets
returns an integer AND that it returns a pointer?

Anyway, it is rubbish since it will error on enything other than an
empty line.

char buf[2048], *h;
if( (h=fgets(buf, sizeof buf, stdin))==buf || h[-1]!='\n')
error();
fgets return buf on input error
in the ok case return the h in buffer that *h=0

This is even worse.

i can not remember all (nor use them much in the direct way) but i
think they are like as the last i wrote

You seem to to have remembered anything since I've not managed to find
anything correct in your post.

 

[Guest]

Harald van D?k said:


If I understand you correctly, you're suggesting that a program could
feed its own output to itself by stuffing it down a pipe and picking it
up with gets().
Correct.

This seems ludicrously pointless.

It's not. If you have a complex text processing routine, often the
easiest way of writing it is to intermix reads and writes. If you have
two such text processing routines, you cannot directly combine them,
if you want the second to act on the output of the first, because it's
not easy to rewrite the first as a function returning the next
character it would output. A pipe is a logical solution for this.

 

[Malcolm McLean]

False dichotomy.


The third is that you have to be awake when you use it, just like you
have to be awake when you use `+` or `if` or `*`.

`fgets` is "dangerous" in the same way that most code is.

You are very nearly right.
All functions or nearly all are dangerous if passed bad data. fgets() is
distinguished because, if used improperly, it can be a source of corrupted
data fed to other functions. For instance a patient record with the last
digit of the weight field cut off, fed to a function that calculates his
drug dose, all because someone didn't check for that trailing newline.

 

[Malcolm McLean]

And fgets() is in practise too difficult for any but the best programmers
to
use safely.

That certainly means I am unable to use fgets() safely,
and since I just wrote this function which reads from stdin
using fgets() I thought I would to like to hear any opinions
on my code from the people here.
Although it may not be the best way to handle memory, I'm
mostly interested to know if this code is correct, and safe.
Thanks!

/Michael Brennan


#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define BUFSZ 128
char *readstr(void)
{
char buffer[BUFSZ];
char *input;
size_t size;
char *tmp;

input = NULL;
size = 0;
do {
tmp = realloc(input, size + BUFSZ);
if (tmp == NULL) {
free(input);
return NULL;
}
input = tmp;

if (fgets(buffer, sizeof buffer, stdin) == NULL) {
free(input);
return NULL;
}

if (size == 0)
strcpy(input, buffer);
else
strcat(input, buffer);

size = strlen(input);
} while (strrchr(buffer, '\n') == NULL);

input[strcspn(input, "\n")] = '\0';

return input;
}

What happens if we pass a line greater than max size_t bytes to your code?

 

[Ian Collins]

You are very nearly right.
All functions or nearly all are dangerous if passed bad data. fgets() is
distinguished because, if used improperly, it can be a source of
corrupted data fed to other functions. For instance a patient record
with the last digit of the weight field cut off, fed to a function that
calculates his drug dose, all because someone didn't check for that
trailing newline.

Just as well you have unit tests to verify the valid range and or number
of characters in your input data.

 

[Ian Collins]

Charlton Wilbur wrote:




I'd be happy around here to be considered competent. On the other hand,
for my "skills assessment" at work, I blithely checked "expert" level
for C. I know my competition.

What's the old saying about the one eyed man in the land of the blind?

 

[Flash Gordon]

Ian Collins wrote, On 27/04/07 21:42:

Just as well you have unit tests to verify the valid range and or number
of characters in your input data.

I would expect it to be caught in the design review myself, when the
reviewer would ask what it is to do if the line is too long. Then, of
course, the requirement specification would have to be updated to
include the missing information on error handling and the people who
reviewed the requirements get kicked for not having spotted the problem.

Well, for the sort of SW Malcolm is talking about I would. For several
degrees of less critical SW I would still expect it to be caught in
either design or code review.

 

[Dave Vandervies]

What happens if we pass a line greater than max size_t bytes to your code?

Your computer vanishes in a puff of logic.


dave

 

[Malcolm McLean]

Your computer vanishes in a puff of logic.

You can generate a text file longer than 4GB and containing no newlines. A
standard PC disk will hold it these days. Then direct it to stdin and feed
it to the program. I suspect many OSes would choke, but I don't think that
is guaranteed.

 

[Drew Lawson]

¬a\/b wrote, On 27/04/07 19:45:

something like:

unsigned h;
char buf[2048];
if((h=fgets(buf, sizeof buf, stdin))==0 || buf[h-1]!='\n')
error();

unsigned h;
char buf[2048];
if((h=fgets(buf, sizeof buf, stdin))<=1 || buf[h-2]!='\n')
error();

Still based on a completely incorrect belief of how fgets is defined.

I believe that the poster is trying to talk about the interface he
feels that fgets() *should* have.

It would be useful to have it return the size read, like read()
does. But as discussed elsewhere in this thread, one can easily
impliment a different "get line and return line size" routine based
on fgetc().

 

[Keith Thompson]

¬a\/b wrote, On 27/04/07 08:56:

<snip more stuff based on this completely false statement>

Wrong. fgets is defined by the C standard as returning a null pointer
on failure or a pointer to the start of the buffer on success. I
suggest you go back to school.

Perhaps he was suggesting a possible improvement to fgets(). As it's
actually defined, if you read a very long line into a sufficiently
large buffer, you have to re-scan the buffer (e.g., using strlen()) to
find the end of the string, even though fgets() *could* have
remembered that information and given it to you. He said that fgets()
*should* return the number of characters, not that it does.

Much of what "¬a\/b" writes is relatively incoherent, but this isn't
an example of that phenomenon.

 

[Richard Heathfield]

Walter Roberson said:

I believe he is talking about fork(), in which case you -do- have
control over the input and output because both sides are the same
program (though not the same process.)

Yes, I thought that was what he was talking about too (re-read my
paragraph and I *think* this will be clear).

Forking and communicating between the processes with a pipe is
common for cases where a portion of the work must be done with
different access permissions.

In such circumstances, you're already locked into an API which gives you
far superior information-sharing methods. It seems to me that Harald's
claim is tantamount to saying "yes, you *can* use gets safely, albeit
only in situations where you'd be a fool to use gets". Well, I won't
argue further with such a claim.

 

[Chris Dollin]

You can generate a text file longer than 4GB and containing no newlines. A
standard PC disk will hold it these days. Then direct it to stdin and feed
it to the program.

Why bother with a big text file?

#include <stdio.h>
int main() { while (1) putchar( "!" ); return /* ha! */ 0; }

(Of course you may not be blessed with a system that can pipe programs
together.)

 

[Malcolm McLean]

Why bother with a big text file?

#include <stdio.h>
int main() { while (1) putchar( "!" ); return /* ha! */ 0; }

(Of course you may not be blessed with a system that can pipe programs
together.)

Except it would be this:

int main()
{
for(i=0;i<(size_t) -1;i++)
putchar("!");
/* at this point sytem wraps round */
/* now we can put up to 127 bytes exploit code at an illegal point in
memory*/
printf("Uggleuggle^&*!!runme\n");
return 0;
}

On most systems malloc() will fail before reaching the max allocation, so
the attack only succeeds in generating a correct NULL return, but if objects
are for some reasons restricted in size to a small fraction of total memory
then we've got a potential exploit as the size counter wraps.
Actually we can do a bit better. Put a NUL somewhere in the initial
stream, fgets() will read it, and then you can target the exploit code to
almost wherever you like in memory, assuming that you know how realloc() is
going to rearrange the block when it shrinks it back to 128 bytes.