Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

broken symlinks and non-executable files not ignored in FindProgramInPath() #29940

Closed
Tracked by #57205 ...
chrisd8088 opened this issue Jun 18, 2019 · 1 comment · Fixed by #55504
Closed
Tracked by #57205 ...

broken symlinks and non-executable files not ignored in FindProgramInPath() #29940

chrisd8088 opened this issue Jun 18, 2019 · 1 comment · Fixed by #55504
Labels
area-System.IO help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@chrisd8088
Copy link

I bumped into a difficult-to-debug problem when using Process.Start() on Linux, which ultimately was due to the fact that the FindProgramInPath() internal method used to look for a given executable file in each component of the $PATH environment variable does not ignore broken symlinks and non-executable files, the way Linux shells and other tools such as which do.

Consider the following scenario, where /usr/local/bin/date is a bad symlink which will be found first when searching $PATH; however, normal system utilities ignore the broken symlink and proceed to find and use /bin/date:

$ /bin/ls -lF /bin/date
-rwxr-xr-x 1 root root 100568 Jan 18  2018 /bin/date*
$ /bin/ls -lF /usr/local/bin/date
lrwxrwxrwx 1 root root 10 Jun 18 11:02 /usr/local/bin/date -> /tmp/dummy
$ printenv PATH
/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
$ which date
/bin/date
$ date
Tue Jun 18 17:32:10 EDT 2019

The following example .NET program, though, throws an exception:

using System;
using System.Diagnostics;

namespace Foo
{
    class Program
    {
        static void Main(string[] args)
        {
            ProcessStartInfo procInfo = new ProcessStartInfo("date");
            procInfo.WorkingDirectory = "/tmp";
            procInfo.UseShellExecute = false;
            procInfo.RedirectStandardOutput = true;
            procInfo.RedirectStandardError = true;

            using (Process proc = new Process())
            {
                proc.StartInfo = procInfo;
                proc.Start();
                Console.WriteLine($"out: {proc.StandardOutput.ReadToEnd()}");
                proc.WaitForExit();
            }
        }
    }
}
$ dotnet run
Unhandled Exception: System.ComponentModel.Win32Exception: No such file or directory
   at Interop.Sys.ForkAndExecProcess(String filename, String[] argv, String[] envp, String cwd, Boolean redirectStdin, Boolean redirectStdout, Boolean redirectStderr, Boolean setUser, UInt32 userId, UInt32 groupId, Int32& lpChildPid, Int32& stdinFd, Int32& stdoutFd, Int32& stderrFd, Boolean shouldThrow)
   at System.Diagnostics.Process.StartCore(ProcessStartInfo startInfo)
   at System.Diagnostics.Process.Start()
   at Foo.Program.Main(String[] args)

I believe this is due to the fact that the File.Exists() check in FindProgramInPath() in src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs is insufficient to determine if the path is merely a symlink, or an actual executable file.

Note that the same example C# code above also fails if /usr/local/bin/date is a regular file which lacks an execute permission mode setting, e.g.:

$ sudo touch /usr/local/bin/date
$ ls -lF /usr/local/bin/date
-rw-r--r-- 1 root root 0 Jun 18 14:52 /usr/local/bin/date
$ which date
/bin/date
$ date
Tue Jun 18 17:32:10 EDT 2019
$ dotnet run
Unhandled Exception: System.ComponentModel.Win32Exception: Permission denied
   at Interop.Sys.ForkAndExecProcess(String filename, String[] argv, String[] envp, String cwd, Boolean redirectStdin, Boolean redirectStdout, Boolean redirectStderr, Boolean setUser, UInt32 userId, UInt32 groupId, Int32& lpChildPid, Int32& stdinFd, Int32& stdoutFd, Int32& stderrFd, Boolean shouldThrow)

It would be ideal if, instead of using File.Exists(), these tests (and others used in ResolvePath()) could instead perform stat(2) and readlink(2) calls to check whether the path corresponds to a valid executable file.

While this wouldn't prevent races with file deletion, etc. (which can happen with the existing code, too), under normal circumstances it would bring the .NET parsing of $PATH more in line with other system utilities.

And if that isn't possible, then at least having the code output the path used in the ForkAndExecProcess() call would be superb, because otherwise it's very difficult and frustrating to ascertain why an executable which can be executed without problems from the command line can't be executed by a .NET program.

(In my case, I ended up tracking down the relevant code here, then cut-and-pasting it over into my program and debugging it piece by piece until I realized it was finding a broken symlink from earlier in $PATH. There may be better ways to do that, but regardless, having an error message which output the path that was not found would be incredibly helpful!)

@JeremyKuhne
Copy link
Member

I agree that we should be following the same sort of semantics shells do and we should dump the path we try to launch when we fail.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the Future milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@carlossanlop carlossanlop removed the untriaged New issue has not been triaged by the area owner label Feb 24, 2020
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 12, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 13, 2021
@adamsitnik adamsitnik modified the milestones: Future, 6.0.0 Jul 19, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants