Skip to content
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

[DumpNTLMInfo.py] fix error with 2003 #1630

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

XiaoliChan
Copy link
Contributor

Fix dump ntlm info when the target is 2003
image

After
image

Signed-off-by: XiaoliChan <2209553467@qq.com>
@anadrianmanrique anadrianmanrique added the bug Unexpected problem or unintended behavior label Oct 12, 2023
@gabrielg5
Copy link
Collaborator

Hi @XiaoliChan,

I've checking this PR. I was able to trigger the exception only if the Server has signing disabled
image
Tried with these setting set as Enabled and the example worked fine

┌──(XXXXX㉿YYYYY)-[~/Desktop/impacket/examples]
└─$ python DumpNTLMInfo.py xxx.XXX.xxx.XXX -debug
Impacket v0.12.0.dev1+20231015.203043.419e6f24 - Copyright 2023 Fortra

[+] Impacket Library Installation Path: /home/XXXXX/.local/lib/python3.11/site-packages/impacket
[+] SMBv1 Enabled : True
[+] Prefered Dialect: NT LM 0.12
[+] Server Security : SIGNING_ENABLED | SIGNING_REQUIRED
[+] Max Read Size : 16.25 KB (16644 bytes)
[+] Max Write Size : 16.25 KB (16644 bytes)
[+] Current Time : 2023-10-26 00:24:27.589375+00:00
[+] Name : XXXXXXX
[+] Domain : XXXXXXX
[+] DNS Domain Name : xxxxxxx
[+] DNS Host Name : xxxxxxx
[+] OS : Windows NT 5.2 Build 3790
[+] Null Session : True

Analyzing it a bit more in depth, found that the SecurityMode property is being set in this function (for the SMB1 protocol)

def _wrapper(self, sessionResponse):
sessionResponse['DialectRevision'] = SMB_DIALECT
if self._dialects_parameters['SecurityMode'] & SMB.SECURITY_SIGNATURES_ENABLED:
sessionResponse['SecurityMode'] = SMB2_NEGOTIATE_SIGNING_ENABLED
if self._SignatureRequired:
sessionResponse['SecurityMode'] |= SMB2_NEGOTIATE_SIGNING_REQUIRED
sessionResponse['MaxReadSize'] = self._dialects_parameters['MaxBufferSize']
sessionResponse['MaxWriteSize'] = self._dialects_parameters['MaxBufferSize']
sessionResponse['SystemTime'] = self._to_long_filetime(self._dialects_parameters['LowDateTime'], self._dialects_parameters['HighDateTime'])
sessionResponse['ServerStartTime'] = 0 # SMB1 has not boot time totally
return sessionResponse

So, there's a flow in the function that could end up with SecurityMode not set.
I guess it would be better to handle this scenario directly in this function.

Perhaps by adding

sessionResponse['SecurityMode'] = 0x0

at the beginning?
So everything is contextualized in that funciton... what do you think?

@gabrielg5 gabrielg5 added the waiting for response Further information is needed from people who opened the issue or pull request label Oct 26, 2023
@XiaoliChan
Copy link
Contributor Author

Perhaps by adding

sessionResponse['SecurityMode'] = 0x0

at the beginning? So everything is contextualized in that funciton... what do you think?

That sounds a good idea! ^^

Copy link
Collaborator

@gabrielg5 gabrielg5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just readded those spaces at the end-of-lines to make the PR changes cleaner ;)

examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
examples/DumpNTLMInfo.py Outdated Show resolved Hide resolved
@gabrielg5
Copy link
Collaborator

Hey, awesome!

you can update the PR with that and I'll merge it right away

thanks!!

Signed-off-by: Xiaoli Chan <2209553467@qq.com>
@XiaoliChan
Copy link
Contributor Author

@gabrielg5 Everything is done!

@gabrielg5 gabrielg5 merged commit c0e949f into fortra:master Oct 27, 2023
9 checks passed
@gabrielg5
Copy link
Collaborator

Thanks @XiaoliChan 🚀

abbra pushed a commit to abbra/impacket that referenced this pull request Nov 27, 2023
* [DumpNTLMInfo.py] fix error with 2003

Signed-off-by: XiaoliChan <2209553467@qq.com>

* [DumpNTLMInfo.py] garbrielg5: review I

Signed-off-by: Xiaoli Chan <2209553467@qq.com>

---------

Signed-off-by: XiaoliChan <2209553467@qq.com>
Signed-off-by: Xiaoli Chan <2209553467@qq.com>
@XiaoliChan XiaoliChan deleted the dump-ntlm-info-2003 branch March 6, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected problem or unintended behavior waiting for response Further information is needed from people who opened the issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants