From c5946549a6d1a5c2e99098a225605b7c2257e3e9 Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Sat, 17 Oct 2026 11:09:12 +0200
Subject: [PATCH] Stop PKGMAN_Exec from parsing its arguments

PKGMAN_Exec joined its arguments with spaces and handed the result to
'sh -c', so an argument containing a space was split, and one containing
'$(...)' or ';' was executed.  Such arguments include archive names
taken from downloaded package metadata.

Pass command and arguments to the shell as positional parameters
instead; it still adds the '2>&1', but never parses them.  The tar
version check passed an empty string meaning "no option", which is now
an argument of its own, so it passes nothing instead.

Assisted-by: Generative AI
---
 gap/PackageManager.gi  | 11 ++++++-----
 gap/archive.gi         |  7 ++++---
 tst/PackageManager.tst |  4 ++++
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/gap/PackageManager.gi b/gap/PackageManager.gi
index cc8d217..d94bcff 100644
--- a/gap/PackageManager.gi
+++ b/gap/PackageManager.gi
@@ -186,13 +186,14 @@ function(dir, cmd, args...)
   out := "";
   outstream := OutputTextString(out, true);
 
-  # Execute the command (capture both stdout and stderr)
-  sh := PKGMAN_PathSystemProgram("sh");
-  args := JoinStringsWithSeparator(args, " ");
-  fullcmd := Concatenation(fullcmd, " ", args, " 2>&1");
+  # Execute the command (capture both stdout and stderr).  The shell only adds
+  # the redirection; it gets command and arguments as positional parameters,
+  # so it never parses them.
   # avoids temporary dir problems in stable-4.12
   ChangeDirectoryCurrent(".");
-  code := Process(dir, sh, instream, outstream, ["-c", fullcmd]);
+  code := Process(dir, sh, instream, outstream,
+                  Concatenation(["-c", "exec \"$0\" \"$@\" 2>&1", fullcmd],
+                                args));
   CloseStream(outstream);
 
   if code <> 0 then
diff --git a/gap/archive.gi b/gap/archive.gi
index 2f28f0d..169c0d4 100644
--- a/gap/archive.gi
+++ b/gap/archive.gi
@@ -75,13 +75,14 @@ function(path)
   # Check which version of tar we are using
   tar := PKGMAN_Exec(".", "tar", "--version");
   if StartsWith(tar.output, "tar (GNU tar)") then
-    options := "--warning=none";
+    options := ["--warning=none"];
   else
-    options := "";
+    options := [];
   fi;
 
   # Check contents
-  exec := PKGMAN_Exec(".", "tar", options, "-tf", path);
+  exec := CallFuncList(PKGMAN_Exec,
+                       Concatenation([".", "tar"], options, ["-tf", path]));
   if exec.code <> 0 then
     Info(InfoPackageManager, 1, "Could not inspect tarball contents");
     return fail;
diff --git a/tst/PackageManager.tst b/tst/PackageManager.tst
index fce6be8..493780b 100644
--- a/tst/PackageManager.tst
+++ b/tst/PackageManager.tst
@@ -76,6 +76,10 @@ Error, <cmd> should be a string
 gap> PKGMAN_Exec(".", "xyzabc");
 fail
 
+# PKGMAN_Exec passes arguments through verbatim, without a shell seeing them
+gap> PKGMAN_Exec(".", "printf", "[%s]", "a  b", "$(echo injected)", "").output;
+"[a  b][$(echo injected)][]"
+
 # FINAL TEST
 # (keep this at the end of the file)
 gap> ChangeDirectoryPackageManager(Filename(DirectoryTemporary(), "pkg/"));