diff options
author | Dyami Caliri <dyami@dragonframe.com> | 2014-09-15 14:09:22 -0700 |
---|---|---|
committer | Friedemann Kleint <Friedemann.Kleint@qt.io> | 2017-12-21 15:45:34 +0000 |
commit | ffc8409aa58c04c1dd140001976b55925ac959f6 (patch) | |
tree | b2aea81bfb1714ae6d6576f1646db10581d5d6bf | |
parent | 83fa66b6d23907ccd443344f9865fcf4ad14e3a9 (diff) |
Use AccessCheck for current user effective file permissions
On Windows, QFileInfo.isWritable() was returning true in situations
where the file would only be writable with elevated privileges. Using
AccessCheck instead of GetEffectiveRightsFromAcl to get the correct
results.
Done-with: Friedemann Kleint <Friedemann.Kleint@qt.io>
Done-with: Edward Welbourne <edward.welbourne@qt.io>
Task-number: QTBUG-30148
Change-Id: I7a3468ac069bf782ca312078e3a84107b6cd468c
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
-rwxr-xr-x[-rw-r--r--] | src/corelib/io/qfilesystemengine_win.cpp | 64 | ||||
-rwxr-xr-x[-rw-r--r--] | tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp | 25 |
2 files changed, 74 insertions, 15 deletions
diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp index b01aa55756..c9a56a81cc 100644..100755 --- a/src/corelib/io/qfilesystemengine_win.cpp +++ b/src/corelib/io/qfilesystemengine_win.cpp @@ -178,6 +178,7 @@ static TRUSTEE_W currentUserTrusteeW; static TRUSTEE_W worldTrusteeW; static PSID currentUserSID = 0; static PSID worldSID = 0; +static HANDLE currentUserImpersonatedToken = nullptr; /* Deletes the allocated SIDs during global static cleanup @@ -198,6 +199,11 @@ SidCleanup::~SidCleanup() ::FreeSid(worldSID); worldSID = 0; } + + if (currentUserImpersonatedToken) { + ::CloseHandle(currentUserImpersonatedToken); + currentUserImpersonatedToken = nullptr; + } } Q_GLOBAL_STATIC(SidCleanup, initSidCleanup) @@ -254,6 +260,12 @@ static void resolveLibs() ::CloseHandle(token); } + token = nullptr; + if (::OpenProcessToken(hnd, TOKEN_IMPERSONATE | TOKEN_QUERY | TOKEN_DUPLICATE | STANDARD_RIGHTS_READ, &token)) { + ::DuplicateToken(token, SecurityImpersonation, ¤tUserImpersonatedToken); + ::CloseHandle(token); + } + typedef BOOL (WINAPI *PtrAllocateAndInitializeSid)(PSID_IDENTIFIER_AUTHORITY, BYTE, DWORD, DWORD, DWORD, DWORD, DWORD, DWORD, DWORD, DWORD, PSID*); PtrAllocateAndInitializeSid ptrAllocateAndInitializeSid = (PtrAllocateAndInitializeSid)GetProcAddress(advapiHnd, "AllocateAndInitializeSid"); if (ptrAllocateAndInitializeSid) { @@ -721,15 +733,49 @@ bool QFileSystemEngine::fillPermissions(const QFileSystemEntry &entry, QFileSyst ACCESS_MASK access_mask; TRUSTEE_W trustee; if (what & QFileSystemMetaData::UserPermissions) { // user - data.knownFlagsMask |= QFileSystemMetaData::UserPermissions; - if(ptrGetEffectiveRightsFromAclW(pDacl, ¤tUserTrusteeW, &access_mask) != ERROR_SUCCESS) - access_mask = (ACCESS_MASK)-1; - if(access_mask & ReadMask) - data.entryFlags |= QFileSystemMetaData::UserReadPermission; - if(access_mask & WriteMask) - data.entryFlags|= QFileSystemMetaData::UserWritePermission; - if(access_mask & ExecMask) - data.entryFlags|= QFileSystemMetaData::UserExecutePermission; + // Using AccessCheck because GetEffectiveRightsFromAcl doesn't account for elevation + if (currentUserImpersonatedToken) { + GENERIC_MAPPING mapping = {FILE_GENERIC_READ, FILE_GENERIC_WRITE, FILE_GENERIC_EXECUTE, FILE_ALL_ACCESS}; + PRIVILEGE_SET privileges; + DWORD grantedAccess; + BOOL result; + + data.knownFlagsMask |= QFileSystemMetaData::UserPermissions; + DWORD genericAccessRights = GENERIC_READ; + ::MapGenericMask(&genericAccessRights, &mapping); + + DWORD privilegesLength = sizeof(privileges); + if (::AccessCheck(pSD, currentUserImpersonatedToken, genericAccessRights, + &mapping, &privileges, &privilegesLength, &grantedAccess, &result) && result) { + data.entryFlags |= QFileSystemMetaData::UserReadPermission; + } + + privilegesLength = sizeof(privileges); + genericAccessRights = GENERIC_WRITE; + ::MapGenericMask(&genericAccessRights, &mapping); + if (::AccessCheck(pSD, currentUserImpersonatedToken, genericAccessRights, + &mapping, &privileges, &privilegesLength, &grantedAccess, &result) && result) { + data.entryFlags |= QFileSystemMetaData::UserWritePermission; + } + + privilegesLength = sizeof(privileges); + genericAccessRights = GENERIC_EXECUTE; + ::MapGenericMask(&genericAccessRights, &mapping); + if (::AccessCheck(pSD, currentUserImpersonatedToken, genericAccessRights, + &mapping, &privileges, &privilegesLength, &grantedAccess, &result) && result) { + data.entryFlags |= QFileSystemMetaData::UserExecutePermission; + } + } else { // fallback to GetEffectiveRightsFromAcl + data.knownFlagsMask |= QFileSystemMetaData::UserPermissions; + if (ptrGetEffectiveRightsFromAclW(pDacl, ¤tUserTrusteeW, &access_mask) != ERROR_SUCCESS) + access_mask = ACCESS_MASK(-1); + if (access_mask & ReadMask) + data.entryFlags |= QFileSystemMetaData::UserReadPermission; + if (access_mask & WriteMask) + data.entryFlags|= QFileSystemMetaData::UserWritePermission; + if (access_mask & ExecMask) + data.entryFlags|= QFileSystemMetaData::UserExecutePermission; + } } if (what & QFileSystemMetaData::OwnerPermissions) { // owner data.knownFlagsMask |= QFileSystemMetaData::OwnerPermissions; diff --git a/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp b/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp index f35dab2cad..efb261ce7e 100644..100755 --- a/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp +++ b/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp @@ -64,6 +64,14 @@ #define Q_NO_SYMLINKS #endif +#if defined(Q_OS_WIN) +QT_BEGIN_NAMESPACE +extern Q_CORE_EXPORT int qt_ntfs_permission_lookup; +QT_END_NAMESPACE +# ifndef Q_OS_WINRT +bool IsUserAdmin(); +# endif +#endif #if defined(Q_OS_UNIX) && !defined(Q_OS_VXWORKS) inline bool qt_isEvilFsTypeName(const char *name) @@ -1634,6 +1642,15 @@ void tst_QFileInfo::isWritable() QVERIFY2(fi.exists(), msgDoesNotExist(fi.absoluteFilePath()).constData()); QVERIFY(!fi.isWritable()); #endif + +#if defined (Q_OS_WIN) && !defined(Q_OS_WINRT) + QScopedValueRollback<int> ntfsMode(qt_ntfs_permission_lookup); + qt_ntfs_permission_lookup = 1; + QFileInfo fi2(QFile::decodeName(qgetenv("SystemRoot") + "/system.ini")); + QVERIFY(fi2.exists()); + QCOMPARE(fi2.isWritable(), IsUserAdmin()); +#endif + #if defined (Q_OS_QNX) // On QNX /etc is usually on a read-only filesystem QVERIFY(!QFileInfo("/etc/passwd").isWritable()); #elif defined (Q_OS_UNIX) && !defined(Q_OS_VXWORKS) // VxWorks does not have users/groups @@ -1807,7 +1824,7 @@ void tst_QFileInfo::detachingOperations() } #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) -BOOL IsUserAdmin() +bool IsUserAdmin() { BOOL b; SID_IDENTIFIER_AUTHORITY NtAuthority = SECURITY_NT_AUTHORITY; @@ -1825,13 +1842,9 @@ BOOL IsUserAdmin() FreeSid(AdministratorsGroup); } - return(b); + return b != FALSE; } -QT_BEGIN_NAMESPACE -extern Q_CORE_EXPORT int qt_ntfs_permission_lookup; -QT_END_NAMESPACE - #endif // Q_OS_WIN && !Q_OS_WINRT #ifndef Q_OS_WINRT |