Fix shell command injection in desktop openUrl

.github/workflows/build.yml

@@ -25,6 +25,7 @@ jobs:
 
     # Run tests.
     - run: nim c tests/test.nim
+    - run: nim r tests/test_openurl.nim
     - run: nim r -d:useCpu tests/test_cpu_pixels.nim
       if: matrix.os == 'windows-latest'
 

src/windy/platforms/linux/x11.nim

@@ -1395,7 +1395,10 @@ proc setConfig*(appName: string, fileName: string, content: string) =
 
 proc openUrl*(url: string) =
   ## Open a URL in the default browser.
-  discard execShellCmd("xdg-open " & url)
+  let process = startProcess("xdg-open", args = [url],
+    options = {poUsePath, poParentStreams})
+  defer: process.close()
+  discard process.waitForExit()
 
 proc openTempTextFile*(title, text: string) =
   ## Open a text file in the default text editor.

src/windy/platforms/macos/platform.nim

@@ -1,5 +1,5 @@
 import
-  std/[os, strutils, times, unicode, pathnorm],
+  std/[os, osproc, strutils, times, unicode, pathnorm],
   pixie/fileformats/png, pixie/images, utils, vmath,
   ../../[common, internal], macdefs
 
@@ -1572,7 +1572,10 @@ proc openTempTextFile*(title, text: string) =
 
 proc openUrl*(url: string) =
   ## Open a URL in the default web browser.
-  discard execShellCmd("open " & url)
+  let process = startProcess("open", args = ["--", url],
+    options = {poUsePath, poParentStreams})
+  defer: process.close()
+  discard process.waitForExit()
 
 proc fileDialogExtensions(filters: seq[FileDialogFilter]): seq[string] =
   ## Collects unique file extensions without wildcards or dots.

src/windy/platforms/win32/platform.nim

@@ -4,6 +4,8 @@ import
   pixie/fileformats/bmp, pixie/images,
   urlly, utils, vmath, windefs, zippy
 
+from std/winlean import shellExecuteW
+
 when defined(useDirectX):
   {.hint: "Using DirectX backend".}
 elif defined(useVulkan):
@@ -2823,7 +2825,9 @@ proc setConfig*(appName: string, fileName: string, content: string) =
 
 proc openUrl*(url: string) =
   ## Open a URL in the default web browser.
-  discard execShellCmd("start " & url)
+  # Windows resolves URL associations through ShellExecute, not CreateProcess.
+  discard shellExecuteW(0, newWideCString("open"), newWideCString(url),
+    nil, nil, SW_SHOWNORMAL)
 
 proc openTempTextFile*(title, text: string) =
   ## Open a text file in the default text editor.

tests/test_openurl.nim

@@ -0,0 +1,77 @@
+import windy
+
+when defined(windows):
+  import windy/platforms/win32/[utils, windefs]
+
+  var openedUrls: seq[string]
+
+  proc recordOpenUrl(
+    hwnd: HWND,
+    operation, target, parameters, directory: LPCWSTR,
+    showCmd: int32
+  ): HINSTANCE {.stdcall, exportc: "ShellExecuteW".} =
+    # Capture the native API boundary without launching a browser.
+    doAssert hwnd == 0
+    doAssert $operation == "open"
+    doAssert parameters == nil
+    doAssert directory == nil
+    doAssert showCmd == SW_SHOWNORMAL
+    openedUrls.add($target)
+    return 33
+else:
+  import std/[json, os, tempfiles]
+
+  # A copy of this executable stands in for open/xdg-open on PATH.
+  if getEnv("WINDY_OPENURL_RECORD") != "":
+    writeFile(getEnv("WINDY_OPENURL_RECORD"), $(%commandLineParams()))
+    quit(0)
+
+proc main() =
+  let urls = [
+    "https://example.com/",
+    "mailto:test@example.com?subject=Hello&body=World",
+    "https://example.com/?first=1&second=2#fragment",
+    "https://example.com/a b/\"quoted\"/'single'",
+    "https://example.com/$(echo injected > windy-openurl-injected)",
+    "https://example.com/`echo injected > windy-openurl-injected`",
+    "https://example.com/; echo injected > windy-openurl-injected",
+    "https://example.com/| echo injected > windy-openurl-injected",
+    "https://example.com/\necho injected > windy-openurl-injected",
+    "https://example.com/%PATH%/$HOME/!name!/a\\b",
+    "https://example.com/café/日本語/🌍?x=1&y=2"
+  ]
+
+  when defined(windows):
+    for url in urls:
+      openUrl(url)
+    doAssert openedUrls == @urls
+  else:
+    let
+      tempDir = createTempDir("windy-openurl-", "")
+      originalDir = getCurrentDir()
+      originalPath = getEnv("PATH")
+      recordPath = tempDir / "args.json"
+      opener = tempDir / (when defined(macosx): "open" else: "xdg-open")
+    defer:
+      setCurrentDir(originalDir)
+      putEnv("PATH", originalPath)
+      delEnv("WINDY_OPENURL_RECORD")
+      removeDir(tempDir)
+
+    copyFile(getAppFilename(), opener)
+    setFilePermissions(opener, {fpUserRead, fpUserWrite, fpUserExec})
+    setCurrentDir(tempDir)
+    putEnv("PATH", tempDir & ":" & originalPath)
+    putEnv("WINDY_OPENURL_RECORD", recordPath)
+
+    for url in urls:
+      if fileExists(recordPath):
+        removeFile(recordPath)
+      openUrl(url)
+      let expected = when defined(macosx): @["--", url] else: @[url]
+      doAssert parseFile(recordPath) == %expected
+      doAssert not fileExists("windy-openurl-injected")
+
+  echo "Windy openUrl regression test passed"
+
+main()

tests/test_openurl.nims

@@ -0,0 +1,3 @@
+when defined(windows):
+  # Link the test's ShellExecuteW recorder instead of loading shell32.dll.
+  switch("dynlibOverride", "shell32.dll")