-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
some netlogon structs for NetrServerPasswordSet2 #951
Conversation
Interestingly, I have a different code. For example, I changed the Length variable in the NL_TRUST_PASSWORD structure to ULONG, it's not LPWSTR. |
It's likely that ClearNewPassword is NL_TRUST_PASSWORD, and not a PNL_TRUST_PASSWORD. |
ULONG would probably be better, I used 4 null bytes for the LPWSTR, probably has the same effect. [MS-NRPC] mentions it should be a pointer to the NL_TRUST_PASSWORD struct, but i'm not familiar enough with RPC to know if/how that would make a difference. |
According to C706 14.3.11.2 (https://pubs.opengroup.org/onlinepubs/009629399/toc.pdf), NDR represents a top-level reference pointer simply as the representation of its referent. But, if the pointer is marked as unique, it's an ordinary pointer (https://docs.microsoft.com/en-us/windows/win32/rpc/default-pointer-types). Yep, it can be tricky to implement MSRPC methods. They could work even if the implementation is not fully-compliant, but you will face NDR errors in corner cases, or when you nest the structures/arrays, and so on. |
Thanks for the PR @dirkjanm .. indeed interesting for a couple of folks ;). Love to see you guys talking about NDR :P. I'll be playing with this hopefully today. |
It doesn't make a difference. We're giving it an NL_TRUST_PASSWORD struct. It sees it as a pointer. 516 null bytes is 516 null bytes. By the way, the struct is
|
Minor changes and cosmetics, merged.. Game on guys. Thanks @dirkjanm |
Right, it works for the exploit but not for legitimate password changing. The reason for this is that the entire NL_TRUST_PASSWORD buffer is passed in encrypted with the session key. @asolino would it make sense to simply change it to a buffer instead? Like that we can accommodate both options (and for the exploit its just null bytes so differentiating between data and length wouldn't really matter). |
I've put some dirty fixes with a buffer in https://github.com/dirkjanm/CVE-2020-1472/blob/master/restorepassword.py. Also fixed function for AES and py3 where the IV should be bytes. |
512 + 4 = 514? @dirkjanm |
Good catch, fixed it. Did work with 514 too in my tests though... :p |
Hang in there a few mins and I'll commit a similar change |
- So you can encrypt the NL_TRUST_PASSWORD struct yourself and put the result in there. - Related to #951
Give it a try now @dirkjanm |
works, just missing the |
There you go.. thanks @dirkjanm ! |
awesome! thanks for the quick fixes and all you do for this project ❤️ |
still needs helpers and tests but the structs may be useful to some people ;)