From 15d33cf70605f425bbe659c179841773b1e9584e 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 URLs and archive
names taken from PackageInfo.g files fetched from the network.

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

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

diff --git a/gap/PackageManager.gi b/gap/PackageManager.gi
index 59cc537..060b132 100644
--- a/gap/PackageManager.gi
+++ b/gap/PackageManager.gi
@@ -126,12 +126,13 @@ function(name, interactive...)
 
   # Look for VC repos
   dirs := List(info, i -> ShallowCopy(i.InstallationPath));
-  for vc in [rec(cmd := "git", stflags := "-s", pullflags := "--ff-only"),
-             rec(cmd := "hg", stflags := "", pullflags := "-uy")] do
+  for vc in [rec(cmd := "git", stflags := ["-s"], pullflags := "--ff-only"),
+             rec(cmd := "hg", stflags := [], pullflags := "-uy")] do
     repo := Filename(List(dirs, Directory), Concatenation(".", vc.cmd));
     if repo <> fail then
       dir := repo{[1 .. Length(repo) - Length("/.") - Length(vc.cmd)]};
-      status := PKGMAN_Exec(dir, vc.cmd, "status", vc.stflags);
+      status := CallFuncList(PKGMAN_Exec,
+                             Concatenation([dir, vc.cmd, "status"], vc.stflags));
       if status = fail then
         return false;
       elif status.code = 0 and status.output = "" then
@@ -292,13 +293,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 fdf4e44..69b1d38 100644
--- a/gap/archive.gi
+++ b/gap/archive.gi
@@ -72,13 +72,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 74e301d..75f5172 100644
--- a/tst/PackageManager.tst
+++ b/tst/PackageManager.tst
@@ -70,6 +70,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> PKGMAN_SetCustomPackageDir(Filename(DirectoryTemporary(), "pkg/"));