Merge pull request #70 from zao/fix/check-url-length

fix: refuse to open overly long URLs

Author: Wires77

engine/system/sys_main.h

@@ -50,6 +50,8 @@ class find_c {
 	std::filesystem::directory_iterator iter;
 };
 
+std::string GetWineHostVersion();
+
 // ==========
 // Interfaces
 // ==========
@@ -75,7 +77,7 @@ class sys_IMain {
 	virtual char*	ClipboardPaste() = 0;
 	virtual bool	SetWorkDir(std::filesystem::path const& newCwd = {}) = 0;
 	virtual void	SpawnProcess(std::filesystem::path cmdName, const char* argList) = 0;
-	virtual void	OpenURL(const char* url) = 0;
+	virtual std::optional<std::string> OpenURL(const char* url) = 0;
 	virtual void	Error(const char* fmt, ...) = 0;
 	virtual void	Exit(const char* msg = NULL) = 0;
 	virtual void	Restart() = 0;

engine/system/win/sys_local.h

@@ -37,7 +37,7 @@ class sys_main_c: public sys_IMain {
 	char*	ClipboardPaste();
 	bool	SetWorkDir(std::filesystem::path const& newCwd = {});
 	void	SpawnProcess(std::filesystem::path cmdName, const char* argList);
-	void	OpenURL(const char* url);
+	std::optional<std::string> OpenURL(const char* url); // return value has failure reason
 	void	Error(const char* fmt, ...);
 	void	Exit(const char* msg = NULL);
 	void	Restart();

engine/system/win/sys_macos.mm

@@ -2,10 +2,11 @@
 #include <CoreFoundation/CFBundle.h>
 #include <ApplicationServices/ApplicationServices.h>
 
-void PlatformOpenURL(const char* textUrl)
+const char* PlatformOpenURL(const char* textUrl)
 {
     std::string_view urlView = textUrl;
     CFURLRef url = CFURLCreateWithBytes(nullptr, (const UInt8*)urlView.data(), urlView.size(), kCFStringEncodingUTF8, nullptr);
     LSOpenCFURLRef(url, nullptr);
     CFRelease(url);
+    return nullptr;
 }

engine/system/win/sys_main.cpp

@@ -434,23 +434,58 @@ void sys_main_c::SpawnProcess(std::filesystem::path cmdName, const char* argList
 #endif
 }
 
+std::string GetWineHostVersion()
+{
+#ifdef _WIN32
+	using WineHostVersionFun = void(const char** /*sysname*/, const char** /*release*/);
+	HMODULE mod = GetModuleHandleA("ntdll.dll");
+	if (!mod)
+		return "";
+	auto ptr = GetProcAddress(mod, "wine_get_host_version");
+	if (!ptr)
+		return "";
+	auto fun = (WineHostVersionFun*)ptr;
+	const char* sysname{};
+	const char* release{};
+	fun(&sysname, &release);
+	return sysname ? sysname : "";
+#else
+	return "";
+#endif
+}
+
 #if _WIN32 || __linux__
-void PlatformOpenURL(const char* url)
+const char* PlatformOpenURL(const char* url)
 {
 #ifdef _WIN32
+	const std::string wineHost = GetWineHostVersion();
+	/*
+	Wine has some loosely determined maximum length on how long of an URL
+	can be, so we pick a "safe" maximum and refuse to open anything longer.
+	*/
+	if ((wineHost == "Linux" || wineHost == "Darwin") && strlen(url) > 1500)
+		return AllocString("Did not open URL, length likely too long for the OS.");
 	ShellExecuteA(NULL, "open", url, NULL, NULL, SW_SHOWDEFAULT);
+	return nullptr;
 #else
 #warning LV: URL opening not implemented on this OS.
 	// TODO(LV): Implement URL opening for other OSes.
+	return AllocString("URL opening not implemented on this OS.");
 #endif
 }
 #else
-void PlatformOpenURL(const char* url);
+const char* PlatformOpenURL(const char* url);
 #endif
 
-void sys_main_c::OpenURL(const char* url)
+std::optional<std::string> sys_main_c::OpenURL(const char* url)
 {
-	PlatformOpenURL(url);
+	if (auto err = PlatformOpenURL(url))
+	{
+		std::string ret = err;
+		FreeString(err);
+		return ret;
+	}
+	return {};
 }
 
 // ==============================

engine/system/win/sys_video.cpp

@@ -98,27 +98,6 @@ void sys_IVideo::FreeHandle(sys_IVideo* hnd)
 	delete (sys_video_c*)hnd;
 }
 
-
-static std::string GetWineHostVersion()
-{
-#ifdef _WIN32
-	using WineHostVersionFun = void (const char** /*sysname*/, const char** /*release*/);
-	HMODULE mod = GetModuleHandleA("ntdll.dll");
-	if (!mod)
-		return "";
-	auto ptr = GetProcAddress(mod, "wine_get_host_version");
-	if (!ptr)
-		return "";
-	auto fun = (WineHostVersionFun*)ptr;
-	const char* sysname{};
-	const char* release{};
-	fun(&sysname, &release);
-	return sysname ? sysname : "";
-#else
-	return "";
-#endif
-}
-
 sys_video_c::sys_video_c(sys_IMain* sysHnd)
 	: sys((sys_main_c*)sysHnd)
 {

ui_api.cpp

@@ -100,7 +100,7 @@
 ** ConPrintTable(table[, noRecurse])
 ** ConExecute("<cmd>")
 ** SpawnProcess("<cmdName>"[, "<args>"])
-** OpenURL("<url>")
+** err = OpenURL("<url>")
 ** SetProfiling(isEnabled)
 ** Restart()
 ** Exit(["<message>"])
@@ -1906,7 +1906,10 @@ static int l_OpenURL(lua_State* L)
 	int n = lua_gettop(L);
 	ui->LAssert(L, n >= 1, "Usage: OpenURL(url)");
 	ui->LAssert(L, lua_isstring(L, 1), "OpenURL() argument 1: expected string, got %s", luaL_typename(L, 1));
-	ui->sys->OpenURL(lua_tostring(L, 1));
+	if (auto errMsg = ui->sys->OpenURL(lua_tostring(L, 1))) {
+		lua_pushstring(L, errMsg->c_str());
+		return 1;
+	}
 	return 0;
 }