CVE-2010-2947: LibHX Heap Based Buffer Overflow

The libHX is a well known C library which is used in various popular Linux distributions including Ubuntu, Fedora, Mandrake etc. and other operating systems. On August 2010, Jan Engelhardt reported a heap based memory corruption vulnerability in the HX_split() routine which can be found at src/string.c.

EXPORT_SYMBOL char **HX_split(const char *str, const char *delim,
    int *cp, int max)
{
	/*
	 * @countp can be NULL in case you are not interested in the number of
	 * fields. In either case, you can find out the number of fields by
	 * scanning through the resulting vector.
	 */
	int count = 0;
	char **ret;

	if (cp == NULL)
		cp = &count;
	*cp = 1;
   ...
	if (max == 0)
		max = *cp;
	else if (*cp > max)
		*cp = max;

	ret = malloc(sizeof(char *) * (*cp + 1));
	ret[*cp] = NULL;

	{
		char *seg, *wp = HX_strdup(str), *bg = wp;
		size_t i = 0;

		while (--max > 0) {
			seg      = HX_strsep(&wp, delim);
			ret[i++] = HX_strdup(seg);
		}

		ret[i++] = HX_strdup(wp);
		free(bg);
	}

	return ret;
}

The fourth argument of this routine (represented using ‘max’ integer) is used to set the maximum number of fields in the given string. As you can read, if this is 0 the code will use the counter pointer as the maximum value. Otherwise, it will check that the counter pointer is greater than the value of ‘max’ and use this to allocate enough heap space using malloc(3). Finally, if none of these happen it will directly use ‘cp’ to allocate the required space using malloc(3). However, the actual code that will place the splitted string in the heap allocated ‘ret’ array uses the ‘max’ value as a counter to its ‘while’ loop. This means that in case of a ‘max’ value greater than the actual ‘cp’ (string’s length) the ‘while’ loop will keep writing data beyond the bounds of the heap allocated ‘ret[]’ resulting in memory corruption.
The following patch was applied to fix this bug:

-       if (max == 0)
+       if (max == 0 || *cp < max)
                max = *cp;
        else if (*cp > max)

That simply checks that the maximum number of arguments (the ‘max’ integer) isn’t greater than the string’s counter (the ‘cp’ one).
In addition to this, a new test case call was added in src/tx-string.cpp like this:

static void t_split2(void)
{
       static const char tmp[] = "";
       int c = 0;
       char **a;

       a = HX_split(tmp, " ", &c, 6);
       printf("Got %d fields\n", c);
       HX_zvecfree(a);
}

That does exactly what we saw above. It calls HX_split() passing an emptry string with a ‘max’ value of 6 and a 0 value for ‘cp’.