Comments (9)
Hello,
Thank you for taking the time to report this issue. You are completely right, deny ACEs are ignored by the check.
The Get-ModifiablePath
function is from the original PowerUp
project, I only slightly modified it to better handle some edge cases. Checking those deny ACEs would add to the complexity for a very little gain. Therefore, the choice was made not to implement it and I think it was an acceptable tradeoff.
That being said, that could be an opportunity for me to try and improve this check by adding this functionality. I will see what I can do.
In the meantime, could you simply tell me if this is a situation you encountered in a real environment or you just found out while testing in a lab environment?
from privesccheck.
I tried to resolve this issue with commit b80e910.
There is now a dedicated and generic Get-AclModificationRights
cmdlet that can check the DACL of a File/Directory/Registry Key.
I implemented the following solution:
- I enumerate "deny" ACEs first. If any of them affects the current user's permissions, I save the "restricted" rights in a list.
- I enumerate the "allow" ACEs. For each ACE, I remove the "restricted" rights (list from step 1) from the list of permissions.
- I proceed as I did before. If the resulting list of permissions grants any modification right, I report it.
As a side note, I noticed that I forgot to include the "Delete" right in the list of potentially interesting rights so I took this opportunity to add it.
This solution is not perfect as I had to alter the content of ACE objects (by removing restricted rights) but that's an acceptable tradeoff IMO, especially given the fact that "deny" ACEs are not that common.
Further testing would obviously be appreciated. :)
from privesccheck.
I've encountered this issue in the test environment. That's why agree with you that less complexity is better than this feature possible implementation.
I've reviewed b80e910 commit and I think there might be an issue:
$UserIdentity = [System.Security.Principal.WindowsIdentity]::GetCurrent()
$CurrentUserSids = $UserIdentity.Groups
$CurrentUserSids
contains only allow
token's SIDs, that's why if()
statement below will always resolve to true
. Thus, step 2 (remove the "restricted" rights from the list of permissions) will never be executed.
$IdentityReferenceSid = Convert-NameToSid -Name $DenyAce.IdentityReference
if ($CurrentUserSids -notcontains $IdentityReferenceSid) { continue }$Restrictions =
# next never executes
$Restrictions = $TypeAccessMask.Keys | Where-Object { $DenyAce.$TypeAccessRights.value__ -band $_ } | ForEach-Object { $TypeAccessMask[$_] }
Here is the little POC:
# Current process' token contains 2 deny SIDS - admin and nt local
PS C:\Users\user> whoami /groups
NT AUTHORITY\Local account and member of Administrators group Well-known group S-1-5-114 Group used for deny only
BUILTIN\Administrators Alias S-1-5-32-544 Group used for deny only
# This is the way you fetch current process token SIDS
PS C:\Users\user> $UserIdentity = [System.Security.Principal.WindowsIdentity]::GetCurrent()
PS C:\Users\user> $CurrentUserSids = $UserIdentity.Groups
PS C:\Users\user> $CurrentUserSids.Value
S-1-5-21-4160622650-3572743448-2429229259-513
S-1-1-0
S-1-5-32-545
S-1-5-4
S-1-2-1
S-1-5-11
S-1-5-15
S-1-5-113
S-1-2-0
S-1-5-64-10
# notice that this list doesn't contain admin (S-1-5-32-544) and nt local (S-1-5-114)
I don't know how to get token's deny SIDs in powershell. I suppose AccessCheck() usage may resolve this issue.
from privesccheck.
Thank you very much for testing and reviewing my solution.
I tested only deny ACES, not deny SIDs, hence why I missed this issue, nice catch!
You might be right, calling AccessCheck
might be a smarter solution than rolling my own ACL check logic.
Damn it, I knew this would give me headaches. 🤯
from privesccheck.
So, I realized that although AccessCheck
would be the most reliable option, it would also result in an information loss as I would no longer be able to say which ACE grants which rights.
Instead, I chose to add a simple test case for restricted groups.
$CurrentUserDenySids = [string[]](Get-TokenInformationGroups -InformationClass RestrictedSids | Select-Object -ExpandProperty SID)
# ...
foreach ($DenyAce in $DenyAces) {
# ...
$IdentityReferenceSid = Convert-NameToSid -Name $DenyAce.IdentityReference
if ($CurrentUserDenySids -notcontains $IdentityReferenceSid) { continue }
if ($CurrentUserSids -notcontains $IdentityReferenceSid) { continue }
# ...
}
I have to admit I did only basic testing, especially to ensure that there was no regression, so I don't know whether this solution actually works.
from privesccheck.
Seems valid. Thank you!
from privesccheck.
OK, thank you for your feedback. 👌
from privesccheck.
Hello again,
$CurrentUserDenySids = [string[]](Get-TokenInformationGroups -InformationClass RestrictedSids ...
won't contain deny sids, because, according to MSDN RestrictedSids
will get deny sids of restricted token, and also according to MSDN restricted token is the
access token that has been modified by the CreateRestrictedToken
Thus, if you don't invoke CreateRestrictedToken() you won't get restricted SIDs.
I suggest to use Groups
instead of RestrictedSids
. So this is the correct way to initialize $CurrentUserDenySids
for example above:
PS> $CurrentUserDenySids = Get-TokenInformationGroups -InformationClass Groups | Where-Object {$_.Attributes.Equals("UseForDenyOnly")} | Select-Object -ExpandProperty SID
PS> $CurrentUserDenySids
S-1-5-114 <--- system (deny)
S-1-5-32-544 <--- admin (deny)
from privesccheck.
Hello!
It looks like I missed your message. I updated the script with your proposed solution.
Thank you again! 🙂
from privesccheck.
Related Issues (20)
- Wifi Airstrike Attack (CVE-2021-28316) already mitigated Apr 13, 2021 HOT 2
- Cannot find process with ID for UDP Endpoints HOT 2
- Specified cast is not valid HOT 9
- Windows 11 detected as Windows 10 HOT 3
- Scheduled Task: Binary Exploitation no detection HOT 2
- Empty WinLogon credentials HOT 1
- Check for vulnerable WPAD configuration? HOT 2
- Report Improvement Suggestion HOT 2
- [Improvement] Check for vulnerable drivers HOT 1
- SERVICES > Unquoted Path not work as expected HOT 3
- Credential Guard HOT 3
- [Feature Request] Readd Compliance Field HOT 4
- Getting erros when trying to run it HOT 2
- Get-HotFixList misses updates HOT 2
- LapsV2 is not being checked? HOT 3
- Check SMB signing required HOT 5
- Service binary permissions false positive HOT 1
- List ASR rules HOT 2
- Bug in Find-SccmCacheFileCredentials? HOT 2
- Detect Defender exclusions rules and ASR rules HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from privesccheck.