fix: replace strcpy with strlcpy and update length assertion

[Skyaero42]

• Split from: fix: Replace strcpy with strlcpy for robustness #1712

[xezon]

const size_t strLen = end - start;
WWASSERT(strLen < ARRAY_SIZE(w3d_name));
strlcpy(w3d_name, start, min(ARRAY_SIZE(w3d_name), strLen+1));

I think

[Skyaero42]

ARRAY_SIZE doesn't work with w3d_name because it is a pointer, using W3D_NAME_LEN instead

I removed the min because it is superfluous (the assert fires when ARRAY_SIZE(w3d_name) > strLen+1). In case the assert is disabled (retail), the min risks buffer overflow. Hence I used W3D_NAME_LEN here as well

[xezon]

Oh I see this function gets a buffer and then assumes it is large enough to write. This is bad. According to web search this Get_W3D_Name function is never used. Can we remove it?

[xezon]

strlcpy(w3d_name, start, W3D_NAME_LEN); is definitely wrong, because it means it will stop copying start at null terminator, and not at end where it is supposed to end.

[Skyaero42]

I've undone the changes and will address it in a separate PR. I'm pretty sure we can remove the entire function.

[xezon]

Try this

static void Get_W3D_Name (const char *filename, char *w3d_name, size_t w3d_name_size)
{
	assert(filename);
	assert(w3d_name);

	// Figure out the first character of the name of the file
	// (bypass the path if it was given).
	const char *start = strrchr(filename, '\\');
	if (start)
		++start;					// point to first character after the last backslash
	else
		start = filename;		// point to the start of the filename

	// We don't want to copy the .w3d extension. Find where
	// it occurs.
	const char *end = strrchr(start, '.');
	if (!end)
		end = start + strlen(start);	// point to the null character

	// Copy all characters from start to end (excluding 'end')
	// into the w3d_name buffer. Then capitalize the string.
	size_t num_chars = end - start;
	WWASSERT(num_chars < w3d_name_size);
	strlcpy(w3d_name, start, min(w3d_name_size, num_chars));
	strupr(w3d_name);
}