fix: Cwd taint and path semantics (#1125)

docs/about/changelog.md

@@ -33,6 +33,15 @@ Release history of PerlOnJava. See [Roadmap](roadmap.md) for future plans.
   stage generated nested pure-Perl MakeMaker modules correctly (including
   NetAddr::IP's `-noxs` installation path).
 - Preserve `CORE::__SUB__` from enclosing named subroutines inside sort blocks.
+- Taint `Cwd` results under `-T` (`getcwd`, `cwd`, `fastcwd`, `fastgetcwd`,
+  `abs_path`, `realpath`, `fast_abs_path`, `fast_realpath`), matching standard
+  Perl and restoring Data::Compare's taint-mode plugin guard.
+- Distinguish the logical from the physical current directory in `Cwd`:
+  `cwd` and `fastgetcwd` now report a validated `$ENV{PWD}` while `getcwd` and
+  `fastcwd` stay physical, matching standard Perl on Unix-like platforms.
+- Propagate argument taint through `File::Spec` `canonpath`, `catdir`, and
+  `catfile`, so `rel2abs`/`abs2rel` taint their `Cwd`-derived results under
+  `-T` like standard Perl.
 - Preserve the lifetime of borrowed `+>&=` filehandle aliases, restoring
   `Tie::File::Indexed` file-backed array storage.
 - Add a pure-Perl `JSON::Parse` compatibility layer backed by bundled `JSON::PP`.

src/main/java/org/perlonjava/runtime/perlmodule/FileSpec.java

@@ -23,6 +23,11 @@
  * path handling.
  *
  * <p>Extends {@link PerlModuleBase} to leverage module initialization and method registration.</p>
+ *
+ * <p>The bundled {@code File/Spec/Unix.pm} installs {@code canonpath}, {@code catdir} and
+ * {@code catfile} with {@code *name = \&_pp_name unless defined &name}, so those three keep the
+ * Java implementations below.  Every other sub in that file is defined unconditionally and
+ * therefore shadows the Java method of the same name once {@code File::Spec} is loaded.</p>
  */
 public class FileSpec extends PerlModuleBase {
 
@@ -77,6 +82,33 @@ public static void initialize() {
         }
     }
 
+    /**
+     * Wraps a freshly built path in a {@link RuntimeList}, carrying over the taint of every
+     * path component the caller supplied.
+     *
+     * <p>The Perl originals of these methods build their answer with {@code join}, {@code s///}
+     * and string concatenation, all of which propagate taint, so a tainted component must taint
+     * the result here too.  Only the caller's arguments are considered: none of these methods
+     * consults the operating system, so none of them introduces taint of its own.  The invocant
+     * in {@code args[0]} is a class name or object and is never part of the result.</p>
+     *
+     * @param result The path value produced by the Java implementation.
+     * @param args   The full argument array, including the invocant at index 0.
+     * @return A {@link RuntimeList} holding the possibly tainted result.
+     */
+    private static RuntimeList withArgumentTaint(RuntimeScalar result, RuntimeArray args) {
+        int count = args.size() - 1;
+        if (count <= 0) {
+            return result.getList();
+        }
+        RuntimeScalar[] inputs = new RuntimeScalar[count];
+        for (int i = 0; i < count; i++) {
+            inputs[i] = args.get(i + 1);
+        }
+        // propagateTaint() may hand back a different scalar, so use its return value.
+        return result.propagateTaint(inputs).getList();
+    }
+
     /**
      * Converts a path to a canonical form, removing redundant separators and up-level references.
      *
@@ -92,9 +124,9 @@ public static RuntimeList canonpath(RuntimeArray args, int ctx) {
 
         // Empty string stays empty (Perl 5 behavior)
         if (path.isEmpty()) {
-            return new RuntimeScalar("").getList();
+            return withArgumentTaint(new RuntimeScalar(""), args);
         }
-        
+
         // These Java methods are installed in File::Spec::Unix. Platform
         // subclasses (including File::Spec::Win32) override them in Perl, so
         // their behavior must remain Unix-specific even on a Windows host.
@@ -113,8 +145,8 @@ public static RuntimeList canonpath(RuntimeArray args, int ctx) {
         if (canonPath.isEmpty()) {
             canonPath = ".";
         }
-        
-        return new RuntimeScalar(canonPath).getList();
+
+        return withArgumentTaint(new RuntimeScalar(canonPath), args);
     }
 
     /**
@@ -176,7 +208,10 @@ public static RuntimeList catdir(RuntimeArray args, int ctx) {
         RuntimeArray canonArgs = new RuntimeArray();
         canonArgs.push(new RuntimeScalar("dummy"));
         canonArgs.push(new RuntimeScalar(result.toString()));
-        return canonpath(canonArgs, ctx);
+        String canonical = canonpath(canonArgs, ctx).elements.get(0).toString();
+        // The intermediate scalars above are clean, so the taint of the caller's
+        // components has to be re-applied to the final answer.
+        return withArgumentTaint(new RuntimeScalar(canonical), args);
     }
 
     /**
@@ -193,7 +228,7 @@ public static RuntimeList catfile(RuntimeArray args, int ctx) {
             if (args.size() == 2) {
                 return canonpath(args, ctx);
             }
-            return new RuntimeScalar("").getList();
+            return withArgumentTaint(new RuntimeScalar(""), args);
         }
 
         // Last real arg is the file component; everything before is directories
@@ -219,17 +254,19 @@ public static RuntimeList catfile(RuntimeArray args, int ctx) {
         String filePart = canonpath(fileCanonArgs, ctx).elements.get(0).toString();
 
         // Combine: if dir is empty, just return the file
+        // The catdir/canonpath calls above ran on clean copies, so the caller's
+        // taint is re-applied to whichever combination is returned.
         if (dir.isEmpty()) {
-            return new RuntimeScalar(filePart).getList();
+            return withArgumentTaint(new RuntimeScalar(filePart), args);
         }
-        
+
         // Ensure proper separator between dir and file
         String separator = "/";
         char lastChar = dir.charAt(dir.length() - 1);
         if (lastChar == '/' || lastChar == '\\') {
-            return new RuntimeScalar(dir + filePart).getList();
+            return withArgumentTaint(new RuntimeScalar(dir + filePart), args);
         }
-        return new RuntimeScalar(dir + separator + filePart).getList();
+        return withArgumentTaint(new RuntimeScalar(dir + separator + filePart), args);
     }
 
     /**

src/main/java/org/perlonjava/runtime/perlmodule/Internals.java

@@ -62,6 +62,8 @@ public static void initialize() {
             internals.registerMethod("stack_refcounted", null);
             internals.registerMethod("V", "V", null);
             internals.registerMethod("getcwd", "getcwd", null);
+            internals.registerMethod("logical_cwd", "logical_cwd", null);
+            internals.registerMethod("taint_propagate", "taintPropagate", "@");
             internals.registerMethod("abs_path", "abs_path", ";$");
             // PerlOnJava-only probe: report whether a fully qualified sub
             // name was installed via typeglob assignment (e.g. Exporter
@@ -900,19 +902,128 @@ public static RuntimeList isInitializedStateVariable(RuntimeArray args, int ctx)
      * This provides a native Java implementation that works on all platforms,
      * which Cwd.pm will use instead of shell-based fallbacks.
      *
+     * <p>This is the <em>physical</em> current directory: {@code chdir}
+     * canonicalizes the target, so symlinks such as macOS {@code /tmp ->
+     * /private/tmp} are already resolved.  Cwd.pm aliases {@code getcwd} and
+     * {@code fastcwd} to this method; the logical forms {@code cwd} and
+     * {@code fastgetcwd} use {@link #logical_cwd} instead.
+     *
+     * <p>The path comes from the operating system, so it is tainted under
+     * {@code -T} exactly like {@code Cwd::getcwd} in standard Perl.
+     *
      * @param args Unused arguments
      * @param ctx  The context in which the method is called
      * @return RuntimeScalar with the current working directory path
      */
     public static RuntimeList getcwd(RuntimeArray args, int ctx) {
-        return new RuntimeScalar(RuntimeEnvironment.currentDirectory()).getList();
+        return new RuntimeScalar(RuntimeEnvironment.currentDirectory())
+                .taintFromExternalInput()
+                .getList();
+    }
+
+    /**
+     * Returns the <em>logical</em> current working directory, the value that
+     * {@code Cwd::cwd} and {@code Cwd::fastgetcwd} return in standard Perl.
+     *
+     * <p>On Unix-like systems those two functions shell out to {@code /bin/pwd},
+     * which runs in its default logical mode: it reports {@code $ENV{PWD}} when
+     * that variable can be trusted and otherwise falls back to the
+     * {@code getcwd(3)} answer.  So after {@code cd /tmp} on macOS,
+     * {@code cwd()} yields {@code /tmp} while {@code getcwd()} yields
+     * {@code /private/tmp}.
+     *
+     * <p>{@code $ENV{PWD}} is never trusted blindly — a stale value left behind
+     * by a plain {@code chdir}, or a hostile one, must not make {@code cwd()}
+     * lie.  It is accepted only when it is an absolute path that names the very
+     * same directory as {@code .}, which is decided by
+     * {@link java.nio.file.Files#isSameFile} (a device+inode comparison on
+     * Unix, the same test {@code pwd} performs).  Anything else — unset, empty,
+     * relative, nonexistent, or a different directory — falls back to the
+     * physical path.
+     *
+     * <p>Like {@link #getcwd}, the result describes the operating system's idea
+     * of the current directory and is therefore tainted under {@code -T}.
+     *
+     * @param args Unused arguments
+     * @param ctx  The context in which the method is called
+     * @return RuntimeScalar with the logical current working directory path
+     */
+    public static RuntimeList logical_cwd(RuntimeArray args, int ctx) {
+        String physical = RuntimeEnvironment.currentDirectory();
+        String logical = trustedEnvPwd(physical);
+        return new RuntimeScalar(logical != null ? logical : physical)
+                .taintFromExternalInput()
+                .getList();
+    }
+
+    /**
+     * Returns a copy of a computed value with taint propagated from the supplied
+     * source values.  Bundled Perl modules use this at platform boundaries where
+     * a path is assembled in Perl but the individual string operations have not
+     * retained the source scalar's taint metadata.
+     *
+     * @param args the computed value followed by zero or more taint sources
+     * @param ctx the calling context
+     * @return the computed value, with taint from any source preserved
+     */
+    public static RuntimeList taintPropagate(RuntimeArray args, int ctx) {
+        if (args.size() == 0) {
+            return new RuntimeScalar().getList();
+        }
+        RuntimeScalar result = new RuntimeScalar(args.get(0));
+        for (int i = 1; i < args.size(); i++) {
+            result = result.propagateTaint(args.get(i));
+        }
+        return result.getList();
+    }
+
+    /**
+     * Returns {@code $ENV{PWD}} when it can stand in for the physical current
+     * directory, or {@code null} when it cannot be trusted.
+     *
+     * @param physical the physical current directory
+     * @return the trusted logical path, or {@code null}
+     */
+    private static String trustedEnvPwd(String physical) {
+        RuntimeScalar pwd = GlobalVariable.getGlobalHash("main::ENV").get("PWD");
+        if (pwd == null || !pwd.getDefinedBoolean()) {
+            return null;
+        }
+        String candidate = pwd.toString();
+        // An empty or relative PWD is meaningless as an absolute answer, and a
+        // path with a NUL byte cannot reach the file system at all.
+        if (candidate.isEmpty() || candidate.indexOf('\0') >= 0) {
+            return null;
+        }
+        try {
+            java.nio.file.Path candidatePath = java.nio.file.Paths.get(candidate);
+            if (!candidatePath.isAbsolute()) {
+                return null;
+            }
+            // Same directory? On Unix this compares device and inode numbers,
+            // so any symlinked alias of the current directory is accepted and
+            // any stale or hostile value is rejected.
+            if (java.nio.file.Files.isSameFile(candidatePath, java.nio.file.Paths.get(physical))) {
+                return candidate;
+            }
+        } catch (java.io.IOException | RuntimeException e) {
+            // Nonexistent path, unreadable parent, or an unparseable name:
+            // treat PWD as untrustworthy and use the physical path.
+            return null;
+        }
+        return null;
     }
 
     /**
      * Gets the absolute path of a file or directory, resolving . and .. components.
      * This provides a reliable, platform-independent way to get absolute paths,
      * which Cwd.pm will use instead of Perl-based implementations.
      *
+     * <p>Like the XS {@code Cwd::abs_path}, the resolved path is derived from
+     * the file system, so it is tainted under {@code -T} regardless of whether
+     * the argument was tainted.  Cwd.pm aliases
+     * abs_path/realpath/fast_abs_path/fast_realpath to this method.
+     *
      * @param args The path to resolve (first argument), or "." if not provided
      * @param ctx  The context in which the method is called
      * @return RuntimeScalar with the absolute path, or undef if the path doesn't exist
@@ -925,7 +1036,11 @@ public static RuntimeList abs_path(RuntimeArray args, int ctx) {
         // produced bare "-I" flags and broke Inline's config subprocess.
         if (path.startsWith("jar:")) {
             if (Jar.isJarDirectory(path) || Jar.exists(path)) {
-                return new RuntimeScalar(path).getList();
+                // The embedded library path is echoed back unchanged; it carries
+                // no operating-system data of its own, so only the caller's taint
+                // is propagated.  Inline uses these paths to build -I flags.
+                RuntimeScalar jarPath = new RuntimeScalar(path);
+                return (args.size() > 0 ? jarPath.propagateTaint(args.get(0)) : jarPath).getList();
             }
             return new RuntimeScalar().getList();
         }
@@ -937,7 +1052,9 @@ public static RuntimeList abs_path(RuntimeArray args, int ctx) {
             if (!file.exists()) {
                 return new RuntimeScalar().getList();  // return undef
             }
-            return new RuntimeScalar(file.getCanonicalPath()).getList();
+            return new RuntimeScalar(file.getCanonicalPath())
+                    .taintFromExternalInput()
+                    .getList();
         } catch (java.io.IOException e) {
             return new RuntimeScalar().getList();  // return undef on error
         }

src/main/perl/lib/Cwd.pm

@@ -76,18 +76,49 @@ sub _vms_efs {
 }
 
 
-# PerlOnJava provides Internals::getcwd/abs_path which work on all platforms.
-# Check early to prevent XSLoader from being loaded (which would fail). These
-# aliases intentionally replace the pure-Perl definitions compiled below, so
-# keep them quiet even when a caller has dynamically enabled global warnings.
+# PerlOnJava provides Internals::getcwd/logical_cwd/abs_path, which work on all
+# platforms. Check early to prevent XSLoader from being loaded (which would
+# fail). These aliases intentionally replace the pure-Perl definitions compiled
+# below, so keep them quiet even when a caller has dynamically enabled global
+# warnings.
+#
+# Internals::getcwd     is the PHYSICAL path (symlinks resolved), matching the
+#                       getcwd(3) syscall used by the XS Cwd::getcwd.
+# Internals::logical_cwd is the LOGICAL path: a validated $ENV{PWD} if it names
+#                       the current directory, otherwise the physical path.
+#                       This is what `pwd` reports, which is how standard Perl
+#                       implements cwd() on Unix (see _backtick_pwd below).
+#
+# Which name gets which form is platform-dependent in standard Perl, so follow
+# its %METHOD_MAP table below:
+#   * Unix-like (darwin, linux, *bsd, solaris, ...): cwd() and fastgetcwd() are
+#     _backtick_pwd (logical); getcwd() is XS and fastcwd() walks up with
+#     chdir('..') -- both physical.
+#   * MSWin32/NT/dos/os2/VMS/qnx: all four names are aliases for one
+#     platform-specific function, so there is no logical/physical split.
+#   * cygwin/amigaos: all four names resolve to _backtick_pwd, i.e. all logical.
 {
     no warnings qw(redefine prototype);
     local $^W = 0;
     if (eval { Internals::getcwd(); 1 }) {
-        *getcwd = \&Internals::getcwd;
-        *cwd = sub { Internals::getcwd() };
-        *fastcwd = \&cwd;
-        *fastgetcwd = \&cwd;
+        my $physical = \&Internals::getcwd;
+        my $logical  = defined &Internals::logical_cwd
+                     ? \&Internals::logical_cwd
+                     : $physical;
+
+        if ($^O =~ /\A(?:MSWin32|NT|dos|os2|VMS|qnx)\z/) {
+            # No logical form on these platforms.
+            $logical = $physical;
+        }
+        elsif ($^O eq 'cygwin' or $^O eq 'amigaos') {
+            # Everything goes through `pwd` on these platforms.
+            $physical = $logical;
+        }
+
+        *getcwd     = $physical;
+        *fastcwd    = $physical;
+        *cwd        = $logical;
+        *fastgetcwd = $logical;
     }
     if (eval { Internals::abs_path('.'); 1 }) {
         *abs_path = \&Internals::abs_path;