[SNOW-1156046] fix toctou vulnerability in EasyLogginConfig

[sfc-gh-erojaslizano]

Description

Invert the order, first read the file and then check the permissions.

Checklist

• Code compiles correctly
• Code is formatted according to Coding Conventions
• Created tests which fail without the change (if possible)
• All tests passing (dotnet test)
• Extended the README / documentation, if necessary
• Provide JIRA issue id (if possible) or GitHub issue id in PR name

[sfc-gh-kkatowiczkowalewski]

Unfortunately, it's still prone to race condition because the filepath is used. First, you open the FileStream and then (milliseconds later) you make the check calling _unixOperations.CheckFileHasAnyOfPermissions that opens the file again. It is possible that between these two opens the file under the path is changed: imagine the path to check is ./my.conf, which initially is a symbolic link to /tmp/bad.conf, and milliseconds later it is repointed to /home/user/valid.conf by the bad actor to make the check pass.

One way to do this securely is to open the file once and from this moment operate only on its handle. You can obtain the handle to the file using FileStream.SafeFileHandle and pass it to File.GetUnixFileMode(SafeFileHandle) to obtain permissions, and check them in a way that is not prone to race conditions.

[sfc-gh-erojaslizano]

Hi @sfc-gh-kkatowiczkowalewski I made the change to use the SafeFileHandle but it only works on net8.0 so I make some conditional compilation to work around that.
I'm still working on the updating the failing test

[sfc-gh-knozderko]

We will proceed this way: https://github.com/snowflakedb/SecureUnixLibrary/blob/main/UnixMonoPOC/SecureUnix.cs

[codecov]

Codecov Report

Attention: Patch coverage is 72.41379% with 8 lines in your changes missing coverage. Please review.

Project coverage is 85.13%. Comparing base (982a1e4) to head (58c4cd3).

Files	Patch %	Lines
...lake.Data/Configuration/EasyLoggingConfigParser.cs	66.66%	5 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #925      +/-   ##
==========================================
- Coverage   85.55%   85.13%   -0.42%     
==========================================
  Files         106      106              
  Lines       10777    10782       +5     
  Branches     1032     1033       +1     
==========================================
- Hits         9220     9179      -41     
- Misses       1307     1352      +45     
- Partials      250      251       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sfc-gh-erojaslizano]

I have read the CLA Document and I hereby sign the CLA

[sfc-gh-dstempniak]

Can we exclude Mono.Unix from dependencies for .NET 8? @sfc-gh-knozderko wdyt?

[sfc-gh-erojaslizano]

I tried but there are many other places that uses de Mono.Unix. If this change is needed I prefer to do it in a a new task instead

[sfc-gh-dstempniak]

'hasPermissions' name suggests that the file has permissions specified. Maybe remove negations and if hasPermissions then throw an exception.

[sfc-gh-erojaslizano]

I changed the variable from 'haspermission' to a more clear 'hasGroupOrOtherWritePermissions' and inverted the if statement to throw the exception if true.
this was done in 9a2925b