Tcl Source Code

A stack overflow vulnerability in Tcl nmakehelp.c allows code execution via a crated file

The length of local variable 'szBuffer' in GetVersionFromFile is 100. szBuffer can be controlled via crafted argv.

static const char *
GetVersionFromFile(
    const char *filename,
    const char *match,
    int numdots)
{
    size_t cbBuffer = 100;
    static char szBuffer[100];
    char *szResult = NULL;
    FILE *fp = fopen(filename, "rt");

    if (fp != NULL) {
	/*
	 * Read data until we see our match string.
	 */

	while (fgets(szBuffer, cbBuffer, fp) != NULL) {
	    LPSTR p, q;

	    p = strstr(szBuffer, match);
	    if (p != NULL) {
		/*
		 * Skip to first digit after the match.
		 */

		p += strlen(match);
		while (*p && !isdigit(*p)) {
		    ++p;
		}

		/*
		 * Find ending whitespace.
		 */

		q = p;
		while (*q && (strchr("0123456789.ab", *q)) && ((!strchr(".ab", *q)
			    && (!strchr("ab", q[-1])) || --numdots))) {
		    ++q;
		}

		memcpy(szBuffer, p, q - p); // Bug is here
		szBuffer[q-p] = 0;
		szResult = szBuffer;
		break;
	    }
	}
	fclose(fp);
    }
    return szResult;
}


It turns out that it's easy to eliminate the use of memcpy (or memove) altogether, that should make your analysis tool 100% happy!

Well, let's analyse it.

Since szBuffer is initialized with fgets(), it is guaranteed to be a NULL-terminated string. The two later while's check for "*p" and "*q" resp, so those two while-loops end - worst-case - at the final NULL-character. If 'p' points at szBuffer[0] and 'q' points at szBuffer[99], then - still - the memcpy() does not overflow. I don't see at all how this can lead to code execution.

If you found this by some automatic tool, then please report this as "false alarm" to the tool maintainers.

The only bug I see in this line is that 'memcpy()' should have been a 'memmove()', since p is pointing to the same buffer, it could be overlapping, which is disallowed for 'memcpy()'. That's fixed here: [28ef6c0c741408a2] (but - still - that cannot lead to code execution.