Codefix: Potential unterminated string returned from convert_to_fs.

Converting from UTF-8 to UTF-16 could have resulted in a buffer overflow if the buffer size was exactly the length of the converted string.

Pass string_view/span to convert_from/to_fs instead, and ensure the buffer is terminated. This replaces passing a pointer to the buffer and the buffer size as separate parameters, allowing the compiler to pass both in one parameter.

Removes use of `lengthof()`.

src/music/dmusic.cpp

@@ -1107,7 +1107,7 @@ std::optional<std::string_view> MusicDriver_DMusic::Start(const StringList &parm
 		Debug(driver, 1, "Detected DirectMusic ports:");
 		for (int i = 0; _music->EnumPort(i, &caps) == S_OK; i++) {
 			if (caps.dwClass == DMUS_PC_OUTPUTCLASS) {
-				Debug(driver, 1, " {}: {}{}", i, convert_from_fs(caps.wszDescription, desc, lengthof(desc)), i == pIdx ? " (selected)" : "");
+				Debug(driver, 1, " {}: {}{}", i, convert_from_fs(caps.wszDescription, desc), i == pIdx ? " (selected)" : "");
 			}
 		}
 	}

src/music/win32_m.cpp

@@ -383,7 +383,7 @@ std::optional<std::string_view> MusicDriver_Win32::Start(const StringList &parm)
 			MIDIOUTCAPS moc{};
 			if (midiOutGetDevCaps(tryport, &moc, sizeof(moc)) == MMSYSERR_NOERROR) {
 				char tryportname[128];
-				convert_from_fs(moc.szPname, tryportname, lengthof(tryportname));
+				convert_from_fs(moc.szPname, tryportname);
 
 				/* Compare requested and detected port name.
 				 * If multiple ports have the same name, this will select the last matching port, and the debug output will be confusing. */

src/os/windows/crashlog_win.cpp

@@ -504,14 +504,14 @@ static INT_PTR CALLBACK CrashDialogFunc(HWND wnd, UINT msg, WPARAM wParam, LPARA
 				crash_desc_buf,
 				crash_desc_buf_length,
 				_crash_desc,
-				convert_to_fs(CrashLogWindows::current->crashlog_filename,   filename_buf + filename_buf_length * 0, filename_buf_length),
-				convert_to_fs(CrashLogWindows::current->crashdump_filename,  filename_buf + filename_buf_length * 1, filename_buf_length),
-				convert_to_fs(CrashLogWindows::current->savegame_filename,   filename_buf + filename_buf_length * 2, filename_buf_length),
-				convert_to_fs(CrashLogWindows::current->screenshot_filename, filename_buf + filename_buf_length * 3, filename_buf_length)
+				convert_to_fs(CrashLogWindows::current->crashlog_filename,   {filename_buf + filename_buf_length * 0, filename_buf_length}),
+				convert_to_fs(CrashLogWindows::current->crashdump_filename,  {filename_buf + filename_buf_length * 1, filename_buf_length}),
+				convert_to_fs(CrashLogWindows::current->savegame_filename,   {filename_buf + filename_buf_length * 2, filename_buf_length}),
+				convert_to_fs(CrashLogWindows::current->screenshot_filename, {filename_buf + filename_buf_length * 3, filename_buf_length})
 			);
 
 			SetDlgItemText(wnd, 10, crash_desc_buf);
-			SetDlgItemText(wnd, 11, convert_to_fs(crashlog_dos_nl, crashlog_buf, crashlog_length));
+			SetDlgItemText(wnd, 11, convert_to_fs(crashlog_dos_nl, {crashlog_buf, crashlog_length}));
 			SendDlgItemMessage(wnd, 11, WM_SETFONT, (WPARAM)GetStockObject(ANSI_FIXED_FONT), FALSE);
 			SetWndSize(wnd, -1);
 		} return TRUE;

src/os/windows/font_win32.cpp

@@ -67,7 +67,7 @@ static int CALLBACK EnumFontCallback(const ENUMLOGFONTEX *logfont, const NEWTEXT
 	if ((metric->ntmFontSig.fsCsb[0] & info->locale.lsCsbSupported[0]) == 0 && (metric->ntmFontSig.fsCsb[1] & info->locale.lsCsbSupported[1]) == 0) return 1;
 
 	char font_name[MAX_PATH];
-	convert_from_fs((const wchar_t *)logfont->elfFullName, font_name, lengthof(font_name));
+	convert_from_fs(logfont->elfFullName, font_name);
 
 	info->callback->SetFontNames(info->settings, font_name, &logfont->elfLogFont);
 	if (info->callback->FindMissingGlyphs()) return 1;
@@ -289,12 +289,12 @@ static bool TryLoadFontFromFile(const std::string &font_name, LOGFONT &logfont)
 
 	/* See if this is an absolute path. */
 	if (FileExists(font_name)) {
-		convert_to_fs(font_name, fontPath, lengthof(fontPath));
+		convert_to_fs(font_name, fontPath);
 	} else {
 		/* Scan the search-paths to see if it can be found. */
 		std::string full_font = FioFindFullPath(BASE_DIR, font_name);
 		if (!full_font.empty()) {
-			convert_to_fs(font_name, fontPath, lengthof(fontPath));
+			convert_to_fs(font_name, fontPath);
 		}
 	}
 
@@ -375,7 +375,7 @@ void LoadWin32Font(FontSize fs)
 
 	if (logfont.lfFaceName[0] == 0) {
 		logfont.lfWeight = strcasestr(font_name, " bold") != nullptr ? FW_BOLD : FW_NORMAL; // Poor man's way to allow selecting bold fonts.
-		convert_to_fs(font_name, logfont.lfFaceName, lengthof(logfont.lfFaceName));
+		convert_to_fs(font_name, logfont.lfFaceName);
 	}
 
 	LoadWin32Font(fs, logfont, GetFontCacheFontSize(fs), font_name);

src/os/windows/string_uniscribe.cpp

@@ -147,7 +147,7 @@ static HFONT HFontFromFont(Font *font)
 	logfont.lfHeight = font->fc->GetHeight();
 	logfont.lfWeight = FW_NORMAL;
 	logfont.lfCharSet = DEFAULT_CHARSET;
-	convert_to_fs(font->fc->GetFontName(), logfont.lfFaceName, lengthof(logfont.lfFaceName));
+	convert_to_fs(font->fc->GetFontName(), logfont.lfFaceName);
 
 	return CreateFontIndirect(&logfont);
 }

src/os/windows/win32.cpp

@@ -225,7 +225,7 @@ char *getcwd(char *buf, size_t size)
 {
 	wchar_t path[MAX_PATH];
 	GetCurrentDirectory(MAX_PATH - 1, path);
-	convert_from_fs(path, buf, size);
+	convert_from_fs(path, {buf, size});
 	return buf;
 }
 
@@ -274,7 +274,7 @@ void DetermineBasePaths(const char *exe)
 	} else {
 		/* Use the folder of the config file as working directory. */
 		wchar_t config_dir[MAX_PATH];
-		convert_to_fs(_config_file, path, lengthof(path));
+		convert_to_fs(_config_file, path);
 		if (!GetFullPathName(path, static_cast<DWORD>(std::size(config_dir)), config_dir, nullptr)) {
 			Debug(misc, 0, "GetFullPathName failed ({})", GetLastError());
 			_searchpaths[SP_WORKING_DIR].clear();
@@ -292,7 +292,7 @@ void DetermineBasePaths(const char *exe)
 		_searchpaths[SP_BINARY_DIR].clear();
 	} else {
 		wchar_t exec_dir[MAX_PATH];
-		convert_to_fs(exe, path, std::size(path));
+		convert_to_fs(exe, path);
 		if (!GetFullPathName(path, static_cast<DWORD>(std::size(exec_dir)), exec_dir, nullptr)) {
 			Debug(misc, 0, "GetFullPathName failed ({})", GetLastError());
 			_searchpaths[SP_BINARY_DIR].clear();
@@ -365,37 +365,33 @@ std::wstring OTTD2FS(const std::string &name)
 /**
  * Convert to OpenTTD's encoding from that of the environment in
  * UNICODE. OpenTTD encoding is UTF8, local is wide.
- * @param name pointer to a valid string that will be converted
- * @param utf8_buf pointer to a valid buffer that will receive the converted string
- * @param buflen length in characters of the receiving buffer
- * @return pointer to utf8_buf. If conversion fails the string is of zero-length
+ * @param src wide string that will be converted
+ * @param dst_buf span of valid char buffer that will receive the converted string
+ * @return pointer to dst_buf. If conversion fails the string is of zero-length
  */
-char *convert_from_fs(const wchar_t *name, char *utf8_buf, size_t buflen)
+char *convert_from_fs(const std::wstring_view src, std::span<char> dst_buf)
 {
 	/* Convert UTF-16 string to UTF-8. */
-	int len = WideCharToMultiByte(CP_UTF8, 0, name, -1, utf8_buf, (int)buflen, nullptr, nullptr);
-	if (len == 0) utf8_buf[0] = '\0';
+	int len = WideCharToMultiByte(CP_UTF8, 0, src.data(), static_cast<int>(src.size()), dst_buf.data(), static_cast<int>(dst_buf.size() - 1U), nullptr, nullptr);
+	dst_buf[len] = '\0';
 
-	return utf8_buf;
+	return dst_buf.data();
 }
 
 
 /**
  * Convert from OpenTTD's encoding to that of the environment in
  * UNICODE. OpenTTD encoding is UTF8, local is wide.
- * @param name pointer to a valid string that will be converted
- * @param system_buf pointer to a valid wide-char buffer that will receive the
- * converted string
- * @param buflen length in wide characters of the receiving buffer
- * @param console_cp convert to the console encoding instead of the normal system encoding.
- * @return pointer to system_buf. If conversion fails the string is of zero-length
+ * @param src string that will be converted
+ * @param dst_buf span of valid wide-char buffer that will receive the converted string
+ * @return pointer to dst_buf. If conversion fails the string is of zero-length
  */
-wchar_t *convert_to_fs(const std::string_view name, wchar_t *system_buf, size_t buflen)
+wchar_t *convert_to_fs(const std::string_view src, std::span<wchar_t> dst_buf)
 {
-	int len = MultiByteToWideChar(CP_UTF8, 0, name.data(), (int)name.size(), system_buf, (int)buflen);
-	system_buf[len] = '\0';
+	int len = MultiByteToWideChar(CP_UTF8, 0, src.data(), static_cast<int>(src.size()), dst_buf.data(), static_cast<int>(dst_buf.size() - 1U));
+	dst_buf[len] = '\0';
 
-	return system_buf;
+	return dst_buf.data();
 }
 
 /** Determine the current user's locale. */
@@ -472,8 +468,8 @@ int OTTDStringCompare(std::string_view s1, std::string_view s2)
 	}
 
 	wchar_t s1_buf[512], s2_buf[512];
-	convert_to_fs(s1, s1_buf, lengthof(s1_buf));
-	convert_to_fs(s2, s2_buf, lengthof(s2_buf));
+	convert_to_fs(s1, s1_buf);
+	convert_to_fs(s2, s2_buf);
 
 	return CompareString(MAKELCID(_current_language->winlangid, SORT_DEFAULT), NORM_IGNORECASE, s1_buf, -1, s2_buf, -1);
 }

src/os/windows/win32.h

@@ -12,8 +12,8 @@
 
 bool MyShowCursor(bool show, bool toggle = false);
 
-char *convert_from_fs(const wchar_t *name, char *utf8_buf, size_t buflen);
-wchar_t *convert_to_fs(const std::string_view name, wchar_t *utf16_buf, size_t buflen);
+char *convert_from_fs(const std::wstring_view src, std::span<char> dst_buf);
+wchar_t *convert_to_fs(const std::string_view src, std::span<wchar_t> dst_buf);
 
 void Win32SetCurrentLocaleName(const char *iso_code);
 int OTTDStringCompare(std::string_view s1, std::string_view s2);

src/video/win32_v.cpp

@@ -360,7 +360,7 @@ static LRESULT HandleIMEComposition(HWND hwnd, WPARAM wParam, LPARAM lParam)
 
 			if (len > 0) {
 				static char utf8_buf[1024];
-				convert_from_fs(str.c_str(), utf8_buf, lengthof(utf8_buf));
+				convert_from_fs(str.c_str(), utf8_buf);
 
 				/* Convert caret position from bytes in the input string to a position in the UTF-8 encoded string. */
 				LONG caret_bytes = ImmGetCompositionString(hIMC, GCS_CURSORPOS, nullptr, 0);