Re: [PATCH v2] fuzz: add new oss-fuzz fuzzer for date.c / date.h

[Junio C Hamano <gitster@pobox.com>]

"Arthur Chan via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);

It is somewhat annoying that everybody has to repeat this twice
here, but it is not your fault X-<.

> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> +{
> +	int local;
> +	int num;
> +	uint16_t tz;

tz offset can be negative, so uint16_t is not appropriate.  See
date.c:gm_time_t() that is eventually called from show_date().

> +	char *str;
> +	timestamp_t ts;
> +	enum date_mode_type dmtype;
> +	struct date_mode *dm;
> +
> +	if (size <= 4)
> +		/*
> +		 * we use the first byte to fuzz dmtype and local,
> +		 * then the next three bytes to fuzz tz	offset,
> +		 * and the remainder (at least one byte) is fed
> +		 * as end-user input to approxidate_careful().
> +		 */
> +		return 0;
> +
> +	local = !!(*data & 0x10);
> +	dmtype = (enum date_mode_type)(*data % DATE_UNIX);
> +	if (dmtype == DATE_STRFTIME)
> +		/*
> +		 * Currently DATE_STRFTIME is not supported.
> +		 */
> +		return 0;

There is an off-by-one error above, as modulo DATE_UNIX will never
yield DATE_UNIX.  Presumably we could do something silly like

	tmp = *data % DATE_UNIX;
	if (DATE_STRFTIME <= tmp)
		tmp++;
	dmtime = (enum date_mode_type)tmp;

to pick values from [0..DATE_UNIX) and then shift everything above
DATE_STRFTIME by one to create a hole there and fill DATE_UNIX at
the same time, without wasting a sample by returning.

> +	data++;
> +	size--;
> +
> +	tz = *data++;
> +	tz = (tz << 8) | *data++;
> +	tz = (tz << 8) | *data++;
> +	size -= 3;

If your tz is 16-bit wide, then we do not have to eat three bytes
here, do we?

You never answered my question on your intention.  Is "tz"
considered attacker controlled (and needs to be fuzzed including
invalid values)?

> +	str = (char *)malloc(size + 1);
> +	if (!str)
> +		return 0;
> +	memcpy(str, data, size);
> +	str[size] = '\0';
> +
> +	ts = approxidate_careful(str, &num);
> +	free(str);
> +
> +	dm = date_mode_from_type(dmtype);
> +	dm->local = local;
> +	show_date(ts, (int16_t)tz, dm);
> +
> +	date_mode_release(dm);
> +
> +	return 0;
> +}
>
> base-commit: dadef801b365989099a9929e995589e455c51fed

Thanks.