[OpenAFS-devel] Re: [OpenAFS] vos: failed to parse date...

Jeffrey Hutzelman jhutz@cmu.edu
Thu, 30 Mar 2006 20:19:28 -0500


On Thursday, March 30, 2006 06:16:04 PM -0500 Jim Rees <rees@umich.edu> 
wrote:

>      code =
>  	sscanf(adate, "%d / %d / %d %d : %d : %d%1s", &month, &day2, &year,
> @@ -528,13 +529,28 @@
>  		   &hour, &min, &c[0]);
>  	if (code != 5) {

It looks like this has always been broken.  Note that the format string
includes %d:%d:%d, indicating it could parse seconds, but there is no
output argument for seconds.  This also means that a form with hours,
minutes, and a trailing colon will be erroneously accepted.

A form with seconds will be rejected, but may also scribble on the stack,
depending on what c points to.  A form with seconds and at least one
trailing non-digit will cause sscanf to dereference a garbage pointer and
write there; if that doesn't cause a crash, then the input will be
accepted, but the seconds will be ignored.

Note the use of the extra %1s argument on the end.  The idea is that for
correct input, you use up the input string before reaching that, and it is
therefore not counted in the return value.  So with the extraneous %d
removed, code == 5 means we got exactly what we wanted and no more.

>  	    hour = min = 0;
> -	    code = sscanf(adate, "%d / %d / %d%1s", &month, &day2, &year, 
&c[0]);
> +	    code =
> +		sscanf(adate, "%d / %d / %d%1s", &month, &day2, &year, &c[0]);
>  	    if (code != 3) {
> -		return -1;
> +		code = -1;

This one is right.  Note that the format string includes 4 fields,
including the dummy, and we check for 3.

>  	    }
>  	}
>      }
>
> +    /* New ISO 8601 (subset) method */
> +
> +    if (code < 0) {
> +	hour = min = sec = 0;
> +	code =
> +	    sscanf(adate, "%d - %d - %d %d : %d : %d", &year, &month, &day2,
> +		   &hour, &min, &sec);
> +	if (code < 3)
> +	    code = -1;
> +    }

This lacks sufficient validation; specifically, it doesn't include a dummy
field to tell if there was extra garbage at the end of the input.

> +
> +    if (code < 0)
> +	return code;
> +
>      if ((year < 0) || (month < 1) || (month > 12) || (day2 < 1) || (day2 
> > 31) ||	/* more or less */
>  	(hour < 0) || (hour > 23) || (min < 0) || (min > 59) || (sec < 0)
>   	|| (sec > 59))


-- Jeff