Re: gets() is dead


"Chris Dollin" <eh@xxxxxxxxxxxxxxxxxxxx> wrote in message news:br0Zh.104561$Zb2.56064@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
Malcolm McLean wrote:


"Chris Dollin" <eh@xxxxxxxxxxxxxxxxxxxx> wrote in message
news:8yBYh.101626$Zb2.84404@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
Malcolm McLean wrote:


"Dave Vandervies" <dj3vande@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote in
message
news:f0to66$uv0$1@xxxxxxxxxxxxxxxxxxxxxxx
In article <PKGdnXIZeOtZwK_bnZ2dnUVZ8sGvnZ2d@xxxxxx>,
Malcolm McLean <regniztar@xxxxxxxxxxxxxx> wrote:

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

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.

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:

What 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;
}

I can't imagine why you think something would be that. (Well, I can,
but the presented options are inappropriate for this newsgroup; I'm
already worried enough about SCORPION STARE.)

On 2007-04-26, Malcolm McLean <regniztar@xxxxxxxxxxxxxx> wrote:

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

Here's the code.

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;
}

It is pretty good stab at using fgets() safely, but in fact it isn't right.
When we supply a line of greater that SIZE_MAX bytes the "size" variable will wrap.
On most machines SIZE_MAX also indicates the total memory available to the machine, so malloc() can only fail long before this point is reached, and the function is OK, as long as the caller remembers to flush the input on fail to deal with the half-read data. However if that is not the case it asks for what is probably a block of zero bytes, then write a string to it, i.e. undefined behaviour. Depending on the realloc internals, by putting strategically-placed NULs in the initial input, which fgets() will then read (another problem with the code) we can then direct a 127-byte exploit string to where we want in memory.
So the answer Michael Brennan's question is, no, the code isn't safe, although it is acceptable for everyday use in a non-security non-portable environment.

--
Free games and programming goodies.
http://www.personal.leeds.ac.uk/~bgy1mm

.

Relevant Pages

  • Interactive i/o (was: gets, fgets, scanf none is safe...)
    ... but it is not a good alternative to fgets. ... it, read the next line, process it, etc, using the same buffer every ... to always leave the field terminating char in the input stream. ...
    (comp.lang.c)
  • Re: char* gets(char* buffer)
    ... >>a bug that allows a buffer overrun. ... > I don't really see the need for a getsor fgets(). ... > I'm parsing data so I just read a char at a time. ... need to see the entire string at once. ...
    (comp.lang.c)
  • Re: gets() is dead
    ... and since I just wrote this function which reads from stdin ... char *readstr ... tmp = realloc; ... strcpy(input, buffer); ...
    (comp.lang.c)
  • Re: gets() is dead
    ... and since I just wrote this function which reads from stdin ... char *readstr ... tmp = realloc; ... strcpy(input, buffer); ...
    (comp.lang.c)
  • Re: can someone help me how to compile this program??
    ... char buffer; ... Which implies fflush() can, in this situation, cause the ... fflushjust "writes" the buffer to the stream. ... fflushing before the fgets is just a guarantee that the writing will ...
    (comp.lang.c)