Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ public static void WriteToNewFileWithOwnerRWPermissions(string path, byte[] data
/// Based on https://stackoverflow.com/questions/45132081/file-permissions-on-linux-unix-with-net-core and on
/// https://github.com/NuGet/NuGet.Client/commit/d62db666c710bf95121fe8f5c6a6cbe01985456f
/// </summary>
/// <remarks>
/// <paramref name="path"/> must not be a symbolic link. The caller (<see cref="FileIOWithRetries.CreateAndWriteToFile"/>
/// and <see cref="FileIOWithRetries.TouchFile"/>) enforces this on non-Windows platforms via a pre-check that throws
/// <see cref="InvalidOperationException"/> before entering the write path.
/// </remarks>
Comment thread
iNinja marked this conversation as resolved.
private static void WriteToNewFileWithOwnerRWPermissionsUnix(string path, byte[] data)
{
int _0600 = Convert.ToInt32("600", 8);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ internal static void CreateAndWriteToFile(string filePath, byte[] data, bool set
{
EnsureParentDirectoryExists(filePath, logger);

if (!SharedUtilities.IsWindowsPlatform())
{
ThrowIfCacheFileIsSymlink(filePath);
}
Comment thread
iNinja marked this conversation as resolved.

logger.LogInformation($"Writing cache file");

TryProcessFile(() =>
Expand Down Expand Up @@ -76,6 +81,12 @@ private static void EnsureParentDirectoryExists(string filePath, TraceSourceLogg
internal static void TouchFile(string filePath, TraceSourceLogger logger)
{
EnsureParentDirectoryExists(filePath, logger);

if (!SharedUtilities.IsWindowsPlatform())
{
ThrowIfCacheFileIsSymlink(filePath);
}

logger.LogInformation($"Touching file...");

TryProcessFile(() =>
Expand All @@ -93,6 +104,33 @@ internal static void TouchFile(string filePath, TraceSourceLogger logger)
}, logger);
}

/// <summary>
/// Throws <see cref="InvalidOperationException"/> if <paramref name="filePath"/> is a
/// symbolic link. Called on non-Windows platforms before writing or touching the cache
/// file to prevent a symlink from being used as a write-anywhere primitive.
/// </summary>
/// <remarks>
/// Uses <see cref="File.GetAttributes(string)"/> which maps to <c>lstat(2)</c> on Unix and
/// therefore returns <see cref="FileAttributes.ReparsePoint"/> for the symlink itself
/// rather than following it to the target. If the path does not yet exist the method
/// returns silently — the caller will create the file normally.
/// </remarks>
private static void ThrowIfCacheFileIsSymlink(string filePath)
{
try
{
if ((File.GetAttributes(filePath) & FileAttributes.ReparsePoint) != 0)
{
throw new InvalidOperationException(
$"The cache file path '{filePath}' is a symbolic link. MSAL cache paths must not be symbolic links.");
}
}
catch (FileNotFoundException)
{
// File does not exist yet — not a symlink, proceed normally.
}
}
Comment thread
iNinja marked this conversation as resolved.

internal static void TryProcessFile(Action action, TraceSourceLogger logger)
{
for (int tryCount = 0; tryCount <= FileLockRetryCount; tryCount++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Identity.Client.Extensions.Msal;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Microsoft.Identity.Client.PlatformsCommon.Shared;
using Microsoft.Identity.Test.Common.Core.Helpers;

namespace Microsoft.Identity.Test.Unit.CacheExtension
{
Expand All @@ -27,6 +28,73 @@ public void TestInitialize()
_logger.Listeners.Add(_testListener);
}

[DoNotRunOnWindows]
[TestMethod]
public void CreateAndWriteToFile_SymlinkAtCachePath_ThrowsInvalidOperationException()
Comment thread
Copilot marked this conversation as resolved.
{
string dir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
Directory.CreateDirectory(dir);
string realTarget = Path.Combine(dir, "real.bin");
string symlinkPath = Path.Combine(dir, "cache.bin");
File.WriteAllBytes(realTarget, Array.Empty<byte>());

#if NET6_0_OR_GREATER
File.CreateSymbolicLink(symlinkPath, realTarget);
#else
// net48 only runs on Windows where this test is already skipped by [DoNotRunOnWindows].
using var proc = Process.Start(new ProcessStartInfo("ln", $"-s \"{realTarget}\" \"{symlinkPath}\""));
proc?.WaitForExit();
#endif

try
{
var ex = AssertException.Throws<InvalidOperationException>(() =>
FileIOWithRetries.CreateAndWriteToFile(
symlinkPath,
new byte[] { 1, 2, 3 },
setChmod600: true,
new TraceSourceLogger(_logger)));

StringAssert.Contains(ex.Message, "symbolic link");
}
finally
{
Directory.Delete(dir, recursive: true);
}
}

[DoNotRunOnWindows]
[TestMethod]
public void TouchFile_SymlinkAtCachePath_ThrowsInvalidOperationException()
Comment thread
Copilot marked this conversation as resolved.
{
string dir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
Directory.CreateDirectory(dir);
string realTarget = Path.Combine(dir, "real.bin");
string symlinkPath = Path.Combine(dir, "cache.bin");
File.WriteAllBytes(realTarget, Array.Empty<byte>());

Comment thread
iNinja marked this conversation as resolved.
#if NET6_0_OR_GREATER
File.CreateSymbolicLink(symlinkPath, realTarget);
#else
using var proc = Process.Start(new ProcessStartInfo("ln", $"-s \"{realTarget}\" \"{symlinkPath}\""));
proc?.WaitForExit();
#endif

try
{
var ex = AssertException.Throws<InvalidOperationException>(() =>
FileIOWithRetries.TouchFile(
symlinkPath,
new TraceSourceLogger(_logger)));

StringAssert.Contains(ex.Message, "symbolic link");
}
finally
{
Directory.Delete(dir, recursive: true);
}
}

[TestMethod]
public async Task Touch_FiresEvent_Async()
{
Expand Down
Loading