Re: [PATCH 1/2] efivarfs: Validate filenames much more aggressively

[Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>]

On Mon, Feb 11, 2013 at 02:28:54PM +0000, Matt Fleming wrote:
> + * Return 1 if 'str' is a valid efivarfs filename of the form,
> + *
> + *	VariableName-12345678-1234-1234-1234-1234567891bc
> + */
> +static int efivarfs_valid_name(const char *str, int len)
> +{
> +	const char *s;
> +	int i, j;
> +	int ranges[2][5] = {
> +		{ 0, 9, 14, 19, 24 },
> +		{ 8, 13, 18, 23, 36 }
> +	};
> +
> +	/*
> +	 * We need a GUID, plus at least one letter for the variable name,
> +	 * plus the '-' separator
> +	 */
> +	if (len < GUID_LEN + 2)
> +		return 0;
> +
> +	s = strchr(str, '-');
> +	if (!s)
> +		return 0;
> +
> +	s++;			/* Skip '-' */
> +
> +	/* Ensure we have enough characters for a GUID */
> +	if (len - (s - str) != GUID_LEN)
> +		return 0;
> +
> +	/*
> +	 * Validate that 's' is of the correct format, e.g.
> +	 *
> +	 *	12345678-1234-1234-1234-123456789abc
> +	 */
> +	for (i = 0; i < 5; i++) {
> +		for (j = ranges[0][i]; j < ranges[1][i]; j++) {
> +			if (hex_to_bin(s[j]) < 0)
> +				return 0;
> +		}
> +
> +		if (j < GUID_LEN && s[j] != '-')
> +			return 0;
> +	}
> +
> +	return 1;

Yecchhh...  How about
	static const char dashes[GUID_LEN] = {
		[8] = 1, [13] = 1, [18] = 1, [23] = 1
	};
	const char *s = str + len - GUID_LEN;
	int i;
	/*
	 * We need a GUID, plus at least one letter for the variable name,
	 * plus the '-' separator
	 */
	if (len < GUID_LEN + 2)
		return 0;

	/* GUID should be right after the first '-' */
	if (s - 1 != strchr(str, '-'))
		return 0;

	/*
	 * Validate that 's' is of the correct format, e.g.
	 *
	 *	12345678-1234-1234-1234-123456789abc
	 */
	for (i = 0; i < GUID_LEN; i++) {
		if (dashes[i]) {
			if (*s++ != '-')
				return 0;
		} else {
			if (!isxdigit(*s++))
				return 0;
		}
	}
	return 1;

instead?