From 23adbd6795d4b0a7fbbb606d58fa82e3df0dabc2 Mon Sep 17 00:00:00 2001
From: Seth Stadick <sstadick@gmail.com>
Date: Thu, 22 Jul 2021 07:54:19 -0600
Subject: [PATCH] cli: force binary existance check

Previously, we were only doing a binary existence check on Windows. And
in fact, the main point there wasn't binary existence, but ensuring we
didn't accidentally resolve a binary name relative to the CWD, which
could result in executing a program one didn't mean to run.

However, it is useful to be able to check whether a binary exists on any
platform when associating a glob with a binary. If the binary doesn't
exist, then the association can fail eagerly and let some other glob
apply.

Closes #1946
---
 crates/cli/src/decompress.rs | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/crates/cli/src/decompress.rs b/crates/cli/src/decompress.rs
index 0ef546b..72eefdd 100644
--- a/crates/cli/src/decompress.rs
+++ b/crates/cli/src/decompress.rs
@@ -132,7 +132,7 @@ impl DecompressionMatcherBuilder {
         A: AsRef<OsStr>,
     {
         let glob = glob.to_string();
-        let bin = resolve_binary(Path::new(program.as_ref()))?;
+        let bin = try_resolve_binary(Path::new(program.as_ref()))?;
         let args =
             args.into_iter().map(|a| a.as_ref().to_os_string()).collect();
         self.commands.push(DecompressionCommand { glob, bin, args });
@@ -421,6 +421,34 @@ impl io::Read for DecompressionReader {
 /// On non-Windows, this is a no-op.
 pub fn resolve_binary<P: AsRef<Path>>(
     prog: P,
+) -> Result<PathBuf, CommandError> {
+    if !cfg!(windows) {
+        return Ok(prog.as_ref().to_path_buf());
+    }
+    try_resolve_binary(prog)
+}
+
+/// Resolves a path to a program to a path by searching for the program in
+/// `PATH`.
+///
+/// If the program could not be resolved, then an error is returned.
+///
+/// The purpose of doing this instead of passing the path to the program
+/// directly to Command::new is that Command::new will hand relative paths
+/// to CreateProcess on Windows, which will implicitly search the current
+/// working directory for the executable. This could be undesirable for
+/// security reasons. e.g., running ripgrep with the -z/--search-zip flag on an
+/// untrusted directory tree could result in arbitrary programs executing on
+/// Windows.
+///
+/// Note that this could still return a relative path if PATH contains a
+/// relative path. We permit this since it is assumed that the user has set
+/// this explicitly, and thus, desires this behavior.
+///
+/// If `check_exists` is false or the path is already an absolute path this
+/// will return immediately.
+fn try_resolve_binary<P: AsRef<Path>>(
+    prog: P,
 ) -> Result<PathBuf, CommandError> {
     use std::env;
 
@@ -433,7 +461,7 @@ pub fn resolve_binary<P: AsRef<Path>>(
     }
 
     let prog = prog.as_ref();
-    if !cfg!(windows) || prog.is_absolute() {
+    if prog.is_absolute() {
         return Ok(prog.to_path_buf());
     }
     let syspaths = match env::var_os("PATH") {