Loading zend blacklist file can fail due to unexpected negative path length

[yiyuaner]

Description

In the file ext/opcache/zend_accelerator_blacklist.c, the function zend_accel_blacklist_loadone has the following code:

static void zend_accel_blacklist_loadone(zend_blacklist *blacklist, char *filename) {
    char buf[MAXPATHLEN + 1];
    int path_length;
    ...
    while (fgets(buf, MAXPATHLEN, fp) != NULL) { 
        path_length = strlen(buf);
        if (path_length > 0 && buf[path_length - 1] == '\n') {
		buf[--path_length] = 0;
		if (path_length > 0 && buf[path_length - 1] == '\r') {
			buf[--path_length] = 0;
		}
	}

	/* Strip ctrl-m prefix */
	pbuf = &buf[0];
	while (*pbuf == '\r') {
		*pbuf++ = 0;
		path_length--;
	}

	/* strip \" */
	if (pbuf[0] == '\"' && pbuf[path_length - 1]== '\"') {
		*pbuf++ = 0;
		path_length -= 2;
	}

	if (path_length == 0) {
		continue;
	}

	/* skip comments */
	if (pbuf[0]==';') {
		continue;
	}

	path_dup = zend_strndup(pbuf, path_length);
    }
}

When path_length = strlen(buf) = 1 and buf[0] ='\"', the following code will reduce path_length by 2:

	if (pbuf[0] == '\"' && pbuf[path_length - 1]== '\"') {
		*pbuf++ = 0;
		path_length -= 2;
	}

Therefore, the call to zend_strndup will pass path_length=-1 as the second argument. Normally, this is dangerous and can lead to buffer overflow. Luckily, inside zend_strndup, the following protection code will cause the program to fail directly:

	if (UNEXPECTED(length + 1 == 0)) {
		zend_error_noreturn(E_ERROR, "Possible integer overflow in memory allocation (1 * %zu + 1)", length);
	}

Still, I think the above case should be explicitly checked and avoided.

PHP Version

github master

Operating System

No response