[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: more improvements for mg



Han Boetes wrote:
> Theo de Raadt wrote:
> > snprintf does not return size_t
>
> sizeof returns size_t


Alright... here is how I see it:

Lets asume for simplicity sake that an int has a range from -128
to 127 and size_t has a range from 0 to 255.

Now there are various ways to do this as I have seen:

	size_t		 ret;
	char             foo[100];

	...
	ret = snprintf(foo, sizeof(foo), "%s", input);
	if ((int)ret == -1 || ret >= sizeof(foo))
	     ...

input size:                  what happens:
--------------------------------------------
illegal input                snprintf returns -1, (int)ret == -1 works ok

0 - 99                       none, cause everything is fine

100 - 127                    ret >= sizeof(foo)

128 - 255                    snprintf returns int, result will wrap in snprintf, compiler doesn't
                             warn against this.

255 - 383                    wraps twice, and result will seem valid.



Or alternatively:

	int		 ret;
	char             foo[100];

	...
	ret = snprintf(foo, sizeof(foo), "%s", input);
	if (ret < 0 || ret >= sizeof(foo))
	     ...

input size:                  what happens:
--------------------------------------------
illegal input                snprintf returns -1, ret < 0 works ok

0 - 99                       none, cause everything is fine

100 - 127                    ret >= sizeof(foo)

128 - 255                    snprintf returns int, result will wrap and returns
      			     negative value, ret < 0 catches this.

255 - 383                    snprintf returns int and wraps twice,
      			     and result will seem valid.


Take a look at /usr/src/lib/libc/stdio/snprintf.c; this situation
seems to go unchecked. AFAICT.


Or the suggestion of someone who mailed me to use long for ret:

	long		 ret;
	char             foo[100];

	...
	ret = snprintf(foo, sizeof(foo), "%s", input);
	if (ret == -1 || ret >= sizeof(foo))
	     ...


input size:                  what happens:
--------------------------------------------
illegal input                snprintf returns -1, ret < 0 works ok

0 - 99                       none, cause everything is fine

100 - 127                    ret >= sizeof(foo)

128 - 255                    snprintf returns int, result will wrap and returns
      			     negative value, ret < 0 catches this.

255 - 383                    snprintf return int and wraps twice,
      			     and result will seem valid.

So that also doesn't help.

So to sum up: the following solution seems the best to me at this
moment, but only if snprintf checks wrapping internally and
returns -1 when that happens.

	int		 ret;
	char             foo[100];

	...
	ret = snprintf(foo, (int)sizeof(foo), "%s", input);
	if (ret == -1 || ret >= sizeof(foo))
	     ...

The manpage does not yet explain what happens when return values
overflow:

RETURN VALUES
     The printf(), fprintf(), sprintf(), vprintf(), vfprintf(), vsprintf(),
     and vasprintf() functions return the number of characters printed (not
     including the trailing `\0' used to end output to strings).

     The snprintf() and vsnprintf() functions return the number of characters
     that would have been output if the size were unlimited (again, not
     including the final `\0').  If an output or encoding error occurs, a val-
     ue of -1 is returned instead.
 

The right final sentence would be:

    If an input, output or encoding error occurs, a value of -1 is
    returned instead.



# Han