win32: ensure len does not cause any overreads

compat/win32/dirent.c DIR *opendir(const char *name) · line 30

We tend to avoid assignments in "if", I think before this change it could have passed, but now that we have a more complex expression it's worth splitting it out. So, we can just move it up to when "int" is declared:

	diff --git a/compat/win32/dirent.c b/compat/win32/dirent.c
	index 52420ec7d4d..bf371cc9714 100644
	--- a/compat/win32/dirent.c
	+++ b/compat/win32/dirent.c
	@@ -23,11 +23,11 @@ DIR *opendir(const char *name)
	 	wchar_t pattern[MAX_PATH + 2]; /* + 2 for '/' '*' */
	 	WIN32_FIND_DATAW fdata;
	 	HANDLE h;
	-	int len;
	+	int len = xutftowcs_path(pattern, name);
	 	DIR *dir;
	 
	 	/* convert name to UTF-16 and check length < MAX_PATH */
	-	if ((len = xutftowcs_path(pattern, name)) < 0)
	+	if (len < 0 || len > MAX_PATH)
	 		return NULL;
	 
	 	/* append optional '/' and wildcard '*' */

But that leaves the question of whether this was just omitted from 0217569bb2d (Win32: Unicode file name support (dirent), 2012-01-14) by mistake?

The comment above the code you're tweaking says we're checking that "length < MAX_PATH", but as we can see 0217569bb2d dropped that condition.

So, was that a bug? And if so why is your check for MAX_PATH different than its check?

Shouldn't yours be (as it did):

	if (len + 2 >= MAX_PATH) {
		errno = ENAMETOOLONG;
		return NULL;
	}

?

Perhaps not, but the commit message should discuss it, i.e. why is the MAX_PATH check now subtly different than the pre-0217569bb2d one.q

compat/win32/dirent.c DIR *opendir(const char *name) · line 30

The documentation for xutftowcs_path() says

/**
* Simplified file system specific variant of xutftowcsn, assumes output
* buffer size is MAX_PATH wide chars and input string is \0-terminated,
* fails with ENAMETOOLONG if input string is too long.
*/

Looking at the implementation it seems it does check the length so I don't think we need this change. I haven't looked into why 0217569bb2d (Win32: Unicode file name support (dirent), 2012-01-14) changed the length check from "len + 2 >= MAX_PATH" though.

Best Wishes

Phillip