Skip to content

Commit

Permalink
Follow symlink chain when probing without reparse point flag (#656)
Browse files Browse the repository at this point in the history 
* Follow symlink chain when probing without reparse point flag

* Refactor symlink tests
  • Loading branch information
@narasamdya @dannyvv
narasamdya authored and dannyvv committed Jul 25, 2019
1 parent 360be81 commit 8621d3e
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 61 deletions.
32 changes: 24 additions & 8 deletions Public/Src/Engine/UnitTests/Processes.Detours/PipExecutorDetoursTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6229,8 +6229,10 @@ public async Task CallDeleteOnOpenedHardlink()
}
}

[FactIfSupported(requiresSymlinkPermission: true)]
public async Task CallDetouredCreateFileWForProbingOnly()
[TheoryIfSupported(requiresSymlinkPermission: true)]
[InlineData(true)]
[InlineData(false)]
public async Task CallDetouredCreateFileWForProbingOnly(bool withReparsePointFlag)
{
var context = BuildXLContext.CreateInstanceForTesting();
var pathTable = context.PathTable;
Expand All @@ -6245,13 +6247,15 @@ public async Task CallDetouredCreateFileWForProbingOnly()
pathTable,
"CreateFileWForProbingOnly.lnk",
"CreateFileWForProbingOnly.txt",
"CallDetouredCreateFileWForProbingOnly",
withReparsePointFlag
? "CallDetouredCreateFileWForSymlinkProbeOnlyWithReparsePointFlag"
: "CallDetouredCreateFileWForSymlinkProbeOnlyWithoutReparsePointFlag",
isDirectoryTest: false,
createSymlink: true,
addCreateFileInDirectoryToDependencies: false,
createFileInDirectory: false,
addFirstFileKind: AddFileOrDirectoryKinds.AsDependency,
addSecondFileOrDirectoryKind: AddFileOrDirectoryKinds.None,
addSecondFileOrDirectoryKind: AddFileOrDirectoryKinds.AsDependency,
makeSecondUntracked: true,
createdInputPaths: createdInputPaths);

Expand All @@ -6270,13 +6274,25 @@ public async Task CallDetouredCreateFileWForProbingOnly()

VerifyNormalSuccess(context, result);

var pathsToFalsify = withReparsePointFlag
? new[] { createdInputPaths["CreateFileWForProbingOnly.txt"] }
: new AbsolutePath[0];

var observationsToVerify = new List<(AbsolutePath abosultePath, RequestedAccess requestedAccess, FileAccessStatus fileAccessStatus)>
{
(createdInputPaths["CreateFileWForProbingOnly.lnk"], RequestedAccess.Probe, FileAccessStatus.Allowed)
};

if (!withReparsePointFlag)
{
observationsToVerify.Add((createdInputPaths["CreateFileWForProbingOnly.txt"], RequestedAccess.Probe, FileAccessStatus.Allowed));
}

VerifyFileAccesses(
context,
result.AllReportedFileAccesses,
new[]
{
(createdInputPaths["CreateFileWForProbingOnly.lnk"], RequestedAccess.Probe, FileAccessStatus.Allowed),
});
observationsToVerify.ToArray(),
pathsToFalsify: pathsToFalsify);
}
}

Expand Down
44 changes: 31 additions & 13 deletions Public/Src/Sandbox/Windows/DetoursServices/DetouredFunctions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,30 @@ static bool IsActionableReparsePointType(_In_ const DWORD reparsePointType)
return reparsePointType == IO_REPARSE_TAG_SYMLINK || reparsePointType == IO_REPARSE_TAG_MOUNT_POINT;
}

/// <summary>
/// Checks if flags or attributes has reparse point.
/// </summary>

static bool AttributesHasReparsePoint(_In_ DWORD dwFlagsAndAttributes)
{
return ((dwFlagsAndAttributes & FILE_FLAG_OPEN_REPARSE_POINT) != 0)
|| ((dwFlagsAndAttributes & FILE_OPEN_REPARSE_POINT) != 0);
}

/// <summary>
/// Checks if Detours should follow symlink chain.
/// </summary>
static bool ShouldFollowSymlinkChain(
_In_ LPCWSTR lpFileName,
_In_ DWORD dwDesiredAccess,
_In_ DWORD dwFlagsAndAttributes)
{
return !IgnoreReparsePoints() // Reparse point should not be ignored.
&& IsReparsePoint(lpFileName) // File is a reparse point.
&& (!WantsProbeOnlyAccess(dwDesiredAccess) // It's not probe-only access.
|| !AttributesHasReparsePoint(dwFlagsAndAttributes)); // It's a probe-only access, but no reparse point flag is passed.
}

/// <summary>
/// Gets the final full path by handle.
/// </summary>
Expand Down Expand Up @@ -691,7 +715,6 @@ static bool EnforceReparsePointAccess(
return false;
}

// Enforce the access only we are not doing directory probing/enumeration.
if (enforceAccess)
{
if (WantsWriteAccess(dwDesiredAccess))
Expand All @@ -706,7 +729,7 @@ static bool EnforceReparsePointAccess(
}
}

if (WantsReadAccess(dwDesiredAccess))
if (WantsReadAccess(dwDesiredAccess) || WantsProbeOnlyAccess(dwDesiredAccess))
{
FileReadContext readContext;
WIN32_FIND_DATA findData;
Expand All @@ -727,7 +750,8 @@ static bool EnforceReparsePointAccess(
(readContext.FileExistence == FileExistence::Existent)
&& IsHandleOrPathToDirectory(INVALID_HANDLE_VALUE, fullPath.c_str(), false);

accessCheck = AccessCheckResult::Combine(accessCheck, policyResult.CheckReadAccess(RequestedReadAccess::Read, readContext));
RequestedReadAccess requestedReadAccess = WantsProbeOnlyAccess(dwDesiredAccess) ? RequestedReadAccess::Probe : RequestedReadAccess::Read;
accessCheck = AccessCheckResult::Combine(accessCheck, policyResult.CheckReadAccess(requestedReadAccess, readContext));
}

if (accessCheck.ShouldDenyAccess())
Expand Down Expand Up @@ -2118,11 +2142,8 @@ HANDLE WINAPI Detoured_CreateFileW(

error = GetLastError();

if (!IgnoreReparsePoints() && IsReparsePoint(lpFileName) && !WantsProbeOnlyAccess(dwDesiredAccess))
if (ShouldFollowSymlinkChain(lpFileName, dwDesiredAccess, dwFlagsAndAttributes))
{
// (1) Reparse point should not be ignored.
// (2) File/Directory is a reparse point.
// (3) Desired access is not probe only.
// Note that handle can be invalid because users can CreateFileW of a symlink whose target is non-existent.

// Even though the process CreateFile the file with FILE_FLAG_OPEN_REPARSE_POINT, we need to follow the chain of symlinks
Expand Down Expand Up @@ -5226,11 +5247,8 @@ NTSTATUS NTAPI Detoured_ZwCreateFile(
return result;
}

if (!IgnoreReparsePoints() && IsReparsePoint(path.GetPathString()) && !WantsProbeOnlyAccess(opContext.DesiredAccess))
if (ShouldFollowSymlinkChain(path.GetPathString(), opContext.DesiredAccess, FileAttributes))
{
// (1) Reparse point should not be ignored.
// (2) File/Directory is a reparse point.
// (3) Desired access is not probe only.
// Note that handle can be invalid because users can CreateFileW of a symlink whose target is non-existent.
NTSTATUS ntStatus;

Expand Down Expand Up @@ -5522,7 +5540,7 @@ NTSTATUS NTAPI Detoured_NtCreateFile(
return result;
}

if (!IgnoreReparsePoints() && IsReparsePoint(path.GetPathString()) && !WantsProbeOnlyAccess(opContext.DesiredAccess))
if (ShouldFollowSymlinkChain(path.GetPathString(), opContext.DesiredAccess, FileAttributes))
{
// (1) Reparse point should not be ignored.
// (2) File/Directory is a reparse point.
Expand Down Expand Up @@ -5792,7 +5810,7 @@ NTSTATUS NTAPI Detoured_ZwOpenFile(
return result;
}

if (!IgnoreReparsePoints() && IsReparsePoint(path.GetPathString()) && !WantsProbeOnlyAccess(opContext.DesiredAccess))
if (ShouldFollowSymlinkChain(path.GetPathString(), opContext.DesiredAccess, OpenOptions))
{
// (1) Reparse point should not be ignored.
// (2) File/Directory is a reparse point.
Expand Down
40 changes: 2 additions & 38 deletions Public/Src/Sandbox/Windows/DetoursTests/Main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,43 +672,6 @@ int CallDetouredCreateFileWWithGenericAllAccess()
return (int)GetLastError();
}

int CallDetouredCreateFileWForProbingOnly()
{
HANDLE hFile = CreateFileW(
L"input\\CreateFileWForProbingOnly.lnk",
0,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
NULL);

if (hFile == INVALID_HANDLE_VALUE)
{
return (int)GetLastError();
}

CloseHandle(hFile);

hFile = CreateFileW(
L"input\\CreateFileWForProbingOnly.lnk",
FILE_READ_ATTRIBUTES | FILE_READ_EA,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
NULL);

if (hFile == INVALID_HANDLE_VALUE)
{
return (int)GetLastError();
}

CloseHandle(hFile);

return (int)GetLastError();
}

int CallDetouredMoveFileExWForRenamingDirectory()
{
DWORD attributes = GetFileAttributesW(L"OutputDirectory\\NewDirectory");
Expand Down Expand Up @@ -1179,7 +1142,6 @@ static void GenericTests(const string& verb)
IF_COMMAND(CallDetouredGetFinalPathNameByHandle);
IF_COMMAND(CallAccessInvalidFile);
IF_COMMAND(CallDetouredCreateFileWWithGenericAllAccess);
IF_COMMAND(CallDetouredCreateFileWForProbingOnly);
IF_COMMAND(CallDetouredMoveFileExWForRenamingDirectory);
IF_COMMAND(CallDetouredSetFileInformationByHandleForRenamingDirectory);
IF_COMMAND(CallDetouredZwSetFileInformationByHandleForRenamingDirectory);
Expand Down Expand Up @@ -1219,6 +1181,8 @@ static void SymlinkTests(const string& verb)
IF_COMMAND(CallAccessJunctionSymlink_Junction);
IF_COMMAND(CallAccessOnChainOfJunctions);
IF_COMMAND(CallDetouredAccessesCreateSymlinkForQBuild);
IF_COMMAND(CallDetouredCreateFileWForSymlinkProbeOnlyWithReparsePointFlag);
IF_COMMAND(CallDetouredCreateFileWForSymlinkProbeOnlyWithoutReparsePointFlag);

#undef IF_COMMAND1
#undef IF_COMMAND2
Expand Down
52 changes: 52 additions & 0 deletions Public/Src/Sandbox/Windows/DetoursTests/SymLinkTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,3 +449,55 @@ int CallDetouredNtCreateFileThatAccessesChainOfSymlinks()

return (int)RtlNtStatusToDosError(status);
}

int CallDetouredCreateFileWForSymlinkProbeOnly(bool withReparsePointFlag)
{
DWORD flagsAndAttributes = FILE_FLAG_BACKUP_SEMANTICS;
flagsAndAttributes = withReparsePointFlag
? flagsAndAttributes | FILE_FLAG_OPEN_REPARSE_POINT
: flagsAndAttributes;

HANDLE hFile = CreateFileW(
L"input\\CreateFileWForProbingOnly.lnk",
0,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
flagsAndAttributes,
NULL);

if (hFile == INVALID_HANDLE_VALUE)
{
return (int)GetLastError();
}

CloseHandle(hFile);

hFile = CreateFileW(
L"input\\CreateFileWForProbingOnly.lnk",
FILE_READ_ATTRIBUTES | FILE_READ_EA,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
flagsAndAttributes,
NULL);

if (hFile == INVALID_HANDLE_VALUE)
{
return (int)GetLastError();
}

CloseHandle(hFile);

return (int)GetLastError();
}

int CallDetouredCreateFileWForSymlinkProbeOnlyWithReparsePointFlag()
{
return CallDetouredCreateFileWForSymlinkProbeOnly(true);
}

int CallDetouredCreateFileWForSymlinkProbeOnlyWithoutReparsePointFlag()
{
return CallDetouredCreateFileWForSymlinkProbeOnly(false);
}
3 changes: 2 additions & 1 deletion Public/Src/Sandbox/Windows/DetoursTests/SymLinkTests.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ int CallCreateSymLinkOnFiles();
int CallCreateSymLinkOnDirectories();
int CallAccessSymLinkOnFiles();
int CallCreateAndDeleteSymLinkOnFiles();
int CallMoveSymLinkOnFilesEnforceChainSymLinkAccesses();
int CallMoveSymLinkOnFilesNotEnforceChainSymLinkAccesses();
int CallAccessSymLinkOnDirectories();
int CallDetouredFileCreateThatAccessesChainOfSymlinks();
Expand All @@ -21,3 +20,5 @@ int CallAccessJunctionSymlink_Real();
int CallAccessJunctionSymlink_Junction();
int CallAccessOnChainOfJunctions();
int CallDetouredAccessesCreateSymlinkForQBuild();
int CallDetouredCreateFileWForSymlinkProbeOnlyWithReparsePointFlag();
int CallDetouredCreateFileWForSymlinkProbeOnlyWithoutReparsePointFlag();
1 change: 0 additions & 1 deletion Public/Src/Sandbox/Windows/DetoursTests/Tests.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ int CallAccessInvalidFile();
int CallGetAttributeNonExistent();
int CallGetAttributeNonExistentInDepDirectory();
int CallDetouredCreateFileWWithGenericAllAccess();
int CallDetouredCreateFileWForProbingOnly();
int CallDetouredMoveFileExWForRenamingDirectory();
int CallDetouredSetFileInformationByHandleForRenamingDirectory();
int CallDetouredZwSetFileInformationByHandleForRenamingDirectory();
Expand Down

0 comments on commit 8621d3e

Please sign in to comment.