Require authorization to start processes

Add an auth token for being able to start a process.
Additionally, it verifies that the file you are attempting
to start is in fact executable.

This is a breaking API change.

Closes #1180

CHANGELOG.md

@@ -7,6 +7,7 @@ All notable changes to the Pony compiler and standard library will be documented
 ### Fixed
 
 - Memory copy bounds for `String.clone` (issue #1289).
+- Security issues in `ProcessMonitor` (issue #1180)
 
 ### Added
 

packages/process/_test.pony

@@ -10,7 +10,8 @@ actor Main is TestList
   fun tag tests(test: PonyTest) =>
     test(_TestStdinStdout)
     test(_TestStderr)
-    test(_TestFileExec)
+    test(_TestFileExecCapabilityIsRequired)
+    test(_TestNonExecutablePathResultsInExecveError)
     test(_TestExpect)
     test(_TestWritevOrdering)
     test(_TestPrintvOrdering)
@@ -30,7 +31,8 @@ class iso _TestStdinStdout is UnitTest
       vars.push("HOME=/")
       vars.push("PATH=/bin")
 
-      let pm: ProcessMonitor = ProcessMonitor(consume notifier, path,
+      let auth = h.env.root as AmbientAuth
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
         consume args, consume vars)
         pm.write("one, two, three")
         pm.done_writing()  // closing stdin allows "cat" to terminate
@@ -59,7 +61,8 @@ class iso _TestStderr is UnitTest
       vars.push("HOME=/")
       vars.push("PATH=/bin")
 
-      let pm: ProcessMonitor = ProcessMonitor(consume notifier, path,
+      let auth = h.env.root as AmbientAuth
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
         consume args, consume vars)
         h.long_test(5_000_000_000)
     else
@@ -69,13 +72,12 @@ class iso _TestStderr is UnitTest
   fun timed_out(h: TestHelper) =>
     h.complete(false)
 
-class iso _TestFileExec is UnitTest
+class iso _TestFileExecCapabilityIsRequired is UnitTest
   fun name(): String =>
-    "process/FileExec"
+    "process/TestFileExecCapabilityIsRequired"
 
   fun apply(h: TestHelper) =>
-    let notifier: ProcessNotify iso = _ProcessClient("",
-      "cat: file_does_not_exist: No such file or directory\n", 1, h)
+    let notifier: ProcessNotify iso = _ProcessClient("", "", 1, h)
     try
       let path = FilePath(h.env.root as AmbientAuth, "/bin/date",
         recover val FileCaps.all().unset(FileExec) end)
@@ -85,7 +87,30 @@ class iso _TestFileExec is UnitTest
       vars.push("HOME=/")
       vars.push("PATH=/bin")
 
-      let pm: ProcessMonitor = ProcessMonitor(consume notifier, path,
+      let auth = h.env.root as AmbientAuth
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
+        consume args, consume vars)
+      h.long_test(5_000_000_000)
+    else
+      h.fail("Could not create FilePath!")
+    end
+
+  fun timed_out(h: TestHelper) =>
+    h.complete(false)
+
+class iso _TestNonExecutablePathResultsInExecveError is UnitTest
+  fun name(): String =>
+    "process/_TestNonExecutablePathResultsInExecveError"
+
+  fun apply(h: TestHelper) =>
+    try
+      let path = _setup_file(h)
+      let args: Array[String] iso = recover Array[String](1) end
+      let vars: Array[String] iso = recover Array[String](2) end
+
+      let auth = h.env.root as AmbientAuth
+      let notifier = _setup_notifier(h, path)
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
         consume args, consume vars)
       h.long_test(5_000_000_000)
     else
@@ -95,6 +120,39 @@ class iso _TestFileExec is UnitTest
   fun timed_out(h: TestHelper) =>
     h.complete(false)
 
+  fun _setup_notifier(h: TestHelper, path: FilePath): ProcessNotify iso^ =>
+    recover object is ProcessNotify
+      let _h: TestHelper = h
+      let _path: FilePath = path
+
+     fun ref failed(process: ProcessMonitor ref, err: ProcessError) =>
+        match err
+        | ExecveError => _h.complete(true)
+        else
+          _h.fail("Unexpected process error")
+          _h.complete(false)
+        end
+        _cleanup()
+
+      fun ref dispose(process: ProcessMonitor ref, child_exit_code: I32) =>
+        _h.fail("Dispose shouldn't have been called.")
+        _h.complete(false)
+        _cleanup()
+
+      fun _cleanup() =>
+        _path.remove()
+    end end
+
+  fun _setup_file(h: TestHelper): FilePath ? =>
+    let tmp_dir = FilePath(h.env.root as AmbientAuth, "/tmp/")
+    let path = FilePath(h.env.root as AmbientAuth,
+      tmp_dir.path + "/" + Path.random(32))
+    let tmp_file = CreateFile(path) as File
+    let mode = FileMode
+    mode.any_exec = false
+    tmp_file.path.chmod(consume mode)
+    path
+
 class iso _TestExpect is UnitTest
   fun name(): String =>
     "process/Expect"
@@ -131,7 +189,8 @@ class iso _TestExpect is UnitTest
       vars.push("HOME=/")
       vars.push("PATH=/bin")
 
-      let pm: ProcessMonitor = ProcessMonitor(consume notifier, path,
+      let auth = h.env.root as AmbientAuth
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
         consume args, consume vars)
         h.long_test(5_000_000_000)
     else
@@ -156,7 +215,8 @@ class iso _TestWritevOrdering is UnitTest
       vars.push("HOME=/")
       vars.push("PATH=/bin")
 
-      let pm: ProcessMonitor = ProcessMonitor(consume notifier, path,
+      let auth = h.env.root as AmbientAuth
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
         consume args, consume vars)
       let params: Array[String] iso = recover Array[String](3) end
       params.push("one")
@@ -188,7 +248,8 @@ class iso _TestPrintvOrdering is UnitTest
       vars.push("HOME=/")
       vars.push("PATH=/bin")
 
-      let pm: ProcessMonitor = ProcessMonitor(consume notifier, path,
+      let auth = h.env.root as AmbientAuth
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
         consume args, consume vars)
       let params: Array[String] iso = recover Array[String](3) end
       params.push("one")

packages/process/auth.pony

@@ -0,0 +1,3 @@
+primitive StartProcessAuth
+  new create(from: AmbientAuth) =>
+    None

packages/process/process_monitor.pony

@@ -18,10 +18,7 @@ use "process"
 use "files"
 
 actor Main
-  let _env: Env
-
   new create(env: Env) =>
-    _env = env
     // create a notifier
     let client = ProcessClient(_env)
     let notifier: ProcessNotify iso = consume client
@@ -36,7 +33,8 @@ actor Main
       vars.push("HOME=/")
       vars.push("PATH=/bin")
       // create a ProcessMonitor and spawn the child process
-      let pm: ProcessMonitor = ProcessMonitor(consume notifier, path,
+      let auth = env.root as AmbientAuth
+      let pm: ProcessMonitor = ProcessMonitor(auth, consume notifier, path,
       consume args, consume vars)
       // write to STDIN of the child process
       pm.write("one, two, three")
@@ -170,6 +168,8 @@ type ProcessError is
   | CapError
   )
 
+type ProcessMonitorAuth is (AmbientAuth | StartProcessAuth)
+
 actor ProcessMonitor
   """
   Forks and monitors a process. Notifies a client about STDOUT / STDERR events.
@@ -197,20 +197,34 @@ actor ProcessMonitor
 
   var _closed: Bool = false
 
-  new create(notifier: ProcessNotify iso, filepath: FilePath,
-    args: Array[String] val, vars: Array[String] val)
+  new create(auth: ProcessMonitorAuth, notifier: ProcessNotify iso,
+    filepath: FilePath, args: Array[String] val, vars: Array[String] val)
   =>
     """
     Create infrastructure to communicate with a forked child process
     and register the asio events. Fork child process and notify our
     user about incoming data via the notifier.
     """
     _notifier = consume notifier
+
+    // We need permission to execute and the
+    // file itself needs to be an executable
     if not filepath.caps(FileExec) then
       _notifier.failed(this, CapError)
       return
     end
 
+    let ok = try
+      FileInfo(filepath).mode.any_exec
+    else
+      false
+    end
+    if not ok then
+      // path is to a non-executable file or that file doesn't exist
+      _notifier.failed(this, ExecveError)
+      return
+    end
+
     ifdef posix then
       try
         (_stdin_read, _stdin_write)   = _make_pipe(_FDCLOEXEC())