nativeopen: fix shell escaping

src/excmds.ts

@@ -87,8 +87,13 @@ import * as Metadata from "@src/.metadata.generated"
 import * as Native from "@src/lib/native"
 import * as TTS from "@src/lib/text_to_speech"
 import * as excmd_parser from "@src/parsers/exmode"
+import * as escape from "@src/lib/escape"
+
+// cmcaine: I have my doubts about this library.
+// Before using it, work out if it does do what you want.
+// The escape library I wrote above may be better.
 import * as shell_quote from "node-shell-quote"
-export let quote = shell_quote
+export const quote = shell_quote
 
 /**
   * This is used to drive some excmd handling in `composite`.
@@ -527,7 +532,12 @@ export async function fixamo() {
 /**
  * Uses the native messenger to open URLs.
  *
- * **Be *seriously* careful with this: you can use it to open any URL you can open in the Firefox address bar.**
+ * **Be *seriously* careful with this:**
+ *
+ * 1. the implementation basically execs `firefox --new-tab <your shell escaped string here>`
+ * 2. you can use it to open any URL you can open in the Firefox address bar,
+ *    including ones that might cause side effects (firefox does not guarantee
+ *    that about: pages ignore query strings).
  *
  * You've been warned.
  *
@@ -590,17 +600,15 @@ export async function nativeopen(...args: string[]) {
                     }
                     firefoxArgs.push("--new-tab")
                 }
-                let escapedUrl = url
+                let escapedUrl: string
-                // On linux, we need to quote and escape single quotes in the
+                // We need to quote and escape single quotes in the
                 // url, otherwise an attacker could create an anchor with a url
                 // like 'file:// && $(touch /tmp/dead)' and achieve remote code
                 // execution when the user tries to follow it with `hint -W tabopen`
-                // But windows treats single quotes as "open this file from the
-                // user's directory", so we need to use double quotes there
                 if (os === "win") {
-                    escapedUrl = `"${escapedUrl.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`
+                    escapedUrl = escape.windows_cmd(url)
                 } else {
-                    escapedUrl = `'${escapedUrl.replace(/'/g, '"\'"')}'`
+                    escapedUrl = escape.sh(url)
                 }
                 await Native.run(`${config.get("browser")} ${firefoxArgs.join(" ")} ${escapedUrl}`)
             }

src/lib/escape.test.ts

@@ -0,0 +1,15 @@
+import * as escape from "@src/lib/escape"
+
+import {testAll} from "@src/lib/test_utils"
+
+testAll(escape.sh, [
+    ['foo&rm -rf', "'foo&rm -rf'"],
+    ['foo&rm -rf\'', "'foo&rm -rf'\\'''"],
+    ['foo&rm -rf\'foo&rm -rf\'', "'foo&rm -rf'\\''foo&rm -rf'\\'''"],
+])
+
+testAll(escape.windows_cmd, [
+    ['foo&rm -rf', "foo^&rm -rf"],
+    ['foo&rm -rf\'', "foo^&rm -rf'"],
+    ['"&whoami', '^"^&whoami'],
+])

src/lib/escape.ts

@@ -0,0 +1,35 @@
+/**
+ * Guarantee that your string is interpreted as a string at its destination.
+ */
+
+/** Wrap with single quotes and replace each inner single quote with '\''
+ *
+ * Backslash does not escape characters within a single quoted string, so it
+ * should be left alone.
+ *
+ * Reference:
+ * https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#Single-Quotes
+ *
+ */
+export const sh = (dangerous: string) =>
+    `'${dangerous.replace(/'/g, "'\\''")}'`
+
+/** Escape every cmd metacharacter
+ *
+ * Windows doesn't create argv for your binary program. Your C library decides
+ * how to quote and unquote, etc. All we can do is ensure that cmd passes it
+ * all to the target program and does nothing else.
+ *
+ * Do that by prefixing every metacharacter with ^
+ *
+ * Reference:
+ * https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
+ *
+ */
+export const windows_cmd = (dangerous: string) =>
+    dangerous.replace(/([()%!^"<>&|])/g, "^$1")
+
+/* Alternative implementation that prefixes all characters with ^
+const windows_cmd2 = (dangerous: string) =>
+    "^" + dangerous.split("").join("^")
+*/