dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.88k stars 4.63k forks source link

FileSystemGlobbing does not support specifying absolute paths. #62333

Open Danielku15 opened 2 years ago

Danielku15 commented 2 years ago

Description

FileSystemGlobbing only works with relative paths, if you supply absolute paths, they are not matched. This is problematic if you allow users to supply files through commandline to your application. As a user you would expect you can specify absolute paths, relative paths (both optionally with globbing).

Reproduction Steps

var dllMatcher = new Matcher();
dllMatcher.AddInclude("C:\\Windows\\*.exe"); // absolute
dllMatcher.AddInclude("*.exe"); // relative
var results = dllMatcher.Execute(new DirectoryInfoWrapper(new DirectoryInfo(Environment.CurrentDirectory)));
// results does not contain anything from C:\Windows

Expected behavior

When patterns are specifying absolute paths, it should match results outside the given base directory.

Actual behavior

Only files within the given base directory are considered as candidates and therefore any absolute file paths supplied, are not included in the result.

Regression?

Unknown.

Known Workarounds

No known one.

Configuration

Which version of .NET is the code running on? .net 6.0

What OS and version, and what distro if applicable? Edition Windows 10 Enterprise Version 20H2 Installed on ‎25.‎09.‎2020 OS build 19042.1348 Experience Windows Feature Experience Pack 120.2212.3920.0

What is the architecture (x64, x86, ARM, ARM64)? x64

Do you know whether it is specific to that configuration? No it is not.

If you're using Blazor, which web browser(s) do you see this issue in? Not applicable.

Other information

It might imply a security risk to allow globbing outside the base path so there might be a need for a setting.

From what I've seen the logic in the globbing is:

  1. Enumerate candidates from the input directory
  2. Match them against the globbing patterns.

This is likely where we need to extend/adapt the logic in a way that respects potentially global paths.

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-system-io See info in area-owners.md if you want to be subscribed.

Issue Details
### Description FileSystemGlobbing only works with relative paths, if you supply absolute paths, they are not matched. This is problematic if you allow users to supply files through commandline to your application. As a user you would expect you can specify absolute paths, relative paths (both optionally with globbing). ### Reproduction Steps ```cs var dllMatcher = new Matcher(); dllMatcher.AddInclude("C:\\Windows\\*.exe"); // absolute dllMatcher.AddInclude("*.exe"); // relative var results = dllMatcher.Execute(new DirectoryInfoWrapper(new DirectoryInfo(Environment.CurrentDirectory))); // results does not contain anything from C:\Windows ``` ### Expected behavior When patterns are specifying absolute paths, it should match results outside the given base directory. ### Actual behavior Only files within the given base directory are considered as candidates and therefore any absolute file paths supplied, are not included in the result. ### Regression? Unknown. ### Known Workarounds No known one. ### Configuration _Which version of .NET is the code running on?_ .net 6.0 _What OS and version, and what distro if applicable?_ Edition Windows 10 Enterprise Version 20H2 Installed on ‎25.‎09.‎2020 OS build 19042.1348 Experience Windows Feature Experience Pack 120.2212.3920.0 _What is the architecture (x64, x86, ARM, ARM64)?_ x64 _Do you know whether it is specific to that configuration?_ No it is not. _If you're using Blazor, which web browser(s) do you see this issue in?_ Not applicable. ### Other information It might imply a security risk to allow globbing outside the base path so there might be a need for a setting. From what I've seen the logic in the globbing is: 1. Enumerate candidates from the input directory 2. Match them against the globbing patterns. This is likely where we need to extend/adapt the logic in a way that respects potentially global paths.
Author: Danielku15
Assignees: -
Labels: `area-System.IO`, `untriaged`
Milestone: -
ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-extensions-filesystem, @maryamariyan See info in area-owners.md if you want to be subscribed.

Issue Details
### Description FileSystemGlobbing only works with relative paths, if you supply absolute paths, they are not matched. This is problematic if you allow users to supply files through commandline to your application. As a user you would expect you can specify absolute paths, relative paths (both optionally with globbing). ### Reproduction Steps ```cs var dllMatcher = new Matcher(); dllMatcher.AddInclude("C:\\Windows\\*.exe"); // absolute dllMatcher.AddInclude("*.exe"); // relative var results = dllMatcher.Execute(new DirectoryInfoWrapper(new DirectoryInfo(Environment.CurrentDirectory))); // results does not contain anything from C:\Windows ``` ### Expected behavior When patterns are specifying absolute paths, it should match results outside the given base directory. ### Actual behavior Only files within the given base directory are considered as candidates and therefore any absolute file paths supplied, are not included in the result. ### Regression? Unknown. ### Known Workarounds No known one. ### Configuration _Which version of .NET is the code running on?_ .net 6.0 _What OS and version, and what distro if applicable?_ Edition Windows 10 Enterprise Version 20H2 Installed on ‎25.‎09.‎2020 OS build 19042.1348 Experience Windows Feature Experience Pack 120.2212.3920.0 _What is the architecture (x64, x86, ARM, ARM64)?_ x64 _Do you know whether it is specific to that configuration?_ No it is not. _If you're using Blazor, which web browser(s) do you see this issue in?_ Not applicable. ### Other information It might imply a security risk to allow globbing outside the base path so there might be a need for a setting. From what I've seen the logic in the globbing is: 1. Enumerate candidates from the input directory 2. Match them against the globbing patterns. This is likely where we need to extend/adapt the logic in a way that respects potentially global paths.
Author: Danielku15
Assignees: -
Labels: `untriaged`, `area-Extensions-FileSystem`
Milestone: -
adamsitnik commented 2 years ago

cc @Jozkee

jozkee commented 2 years ago

I agree it should match absolute paths

When patterns are specifying absolute paths, it should match results outside the given base directory.

This sounds to me like a dynamic scope that increments if a rooted path is used as a pattern. Not sure if we want to add support for that, instead I would suggest that users need to reason about the required scope and use that as the "base" in matcher.Match().

jozkee commented 2 years ago

Regarding the described issue, the code is only reading the first segment in the pattern, not sure why. https://github.com/dotnet/runtime/blob/192db36c1233bee44a2611853d987748ad4c3dd6/src/libraries/Microsoft.Extensions.FileSystemGlobbing/src/Internal/PatternContexts/PatternContextLinearInclude.cs#L28-L31

I changed it to read all segments:

while (true)
{
    onDeclare(Pattern.Segments[Frame.SegmentIndex], IsLastSegment());
    if (Frame.SegmentIndex == Pattern.Segments.Count - 1)
    {
        break;
    }
    Frame.SegmentIndex++;
}

That fixed the reported issue but other cases started to fail. I will need to take a deeper look.