Sanitize NTLM hostname to prevent path traversal and injection

[TristanInSec]

Description

Sanitize server-provided NTLM hostname before use in file paths or content. A rogue SMB/RDP/VNC server can set its NTLM AV_NbComputerName to malicious values (path traversal sequences, null bytes, newlines, format string characters), causing file creation outside ~/.nxc/logs/ or hosts file injection.

Single-line fix: strip all characters except [a-zA-Z0-9_\-.] from self.hostname immediately after enum_host_info() in connection.py.

Fixes GHSA-pv83-jh5v-7q4j and GHSA-qqmq-pwrw-2j2w.

Affected Sinks (all fixed by this single change)

Sink	File	Impact
Credential dump output paths	connection.py:252	Path traversal
DPAPI output	smb.py:2099	Path traversal
SAM/LSA/NTDS output	smb.py (multiple)	Path traversal
RDP screenshots	rdp.py:592,620	Path traversal
VNC screenshots	vnc.py:150	Path traversal
--generate-hosts-file	smb.py:312	Newline injection
--generate-krb5-file	smb.py:317	Newline injection

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Deprecation of feature or functionality
• This change requires a documentation update
• This requires a third party update (such as Impacket, Dploot, lsassy, etc)
• This PR was created with the assistance of AI (list what type of assistance, tool(s)/model(s) in the description)

Setup guide for the review

Bug trigger (path traversal):

1. Run poc-rogue-smb.py (rogue SMB server with NTLM hostname ../../../tmp/evil)
2. Run nxc smb 127.0.0.1 --port 4470 -u guest -p '' --dpapi
3. Observe file created at ~/evil_127.0.0.1_<timestamp> instead of ~/.nxc/logs/dpapi/

Bug trigger (hosts file newline injection):

1. Run rogue SMB server with NTLM hostname containing \n10.13.37.99 dc01.corp.local
2. Run nxc smb 127.0.0.1 --port 4472 -u guest -p '' --generate-hosts-file /tmp/hosts
3. Observe injected entries in /tmp/hosts

Tested on: Kali Linux, Python 3.13, NXC 1.5.0, Impacket 0.14.0.

Checklist:

• I have ran Ruff against my changes (poetry: poetry run ruff check ., use --fix to automatically fix what it can)
• I have added or updated the tests/e2e_commands.txt file if necessary (new modules or features are required to be added to the e2e tests)
• If reliant on changes of third party dependencies, such as Impacket, dploot, lsassy, etc, I have linked the relevant PRs in those projects
• I have linked relevant sources that describes the added technique (blog posts, documentation, etc)
• I have performed a self-review of my own code (not an AI review)
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (PR here: https://github.com/Pennyw0rth/NetExec-Wiki)

[github-actions]

It looks like the PR template may not have been filled out. The following sections appear to be missing:

• Description

• Type of change

• Setup guide for the review

• Checklist

Please edit your PR description to include them. The template helps reviewers understand and test your changes. Thanks!

[NeffIsBack]

Thanks for the bug report! We'll look into it soon.

Regarding the DoS: there are likely plenty of ways a server could respond with broken hostnames or other non-compliant structures that would lead to a crash in impacket. Since it doesn't crash nxc entirely, but just the scan of the host this is imo pretty much the same as just denying an SMB connection (just very noisy).

Also I am not sure if I would like to be so restrictive, since I have no idea what weird hostnames could be out there in the world. Wouldn't bet against the chance some server puts weird chars in their hostname line the dollar symbol or similar. Maybe we should just strip strings ".." or similar, that are pretty much always intentionally bad

[TristanInSec]

Thanks for the bug report! We'll look into it soon.

Regarding the DoS: there are likely plenty of ways a server could respond with broken hostnames or other non-compliant structures that would lead to a crash in impacket. Since it doesn't crash nxc entirely, but just the scan of the host this is imo pretty much the same as just denying an SMB connection (just very noisy).

Also I am not sure if I would like to be so restrictive, since I have no idea what weird hostnames could be out there in the world. Wouldn't bet against the chance some server puts weird chars in their hostname line the dollar symbol or similar. Maybe we should just strip strings ".." or similar, that are pretty much always intentionally bad

Thanks for the review @NeffIsBack

On the DoS: the null byte crash actually kills the entire nxc process, not just the per-host worker. The ValueError: embedded null byte from open() at smb.py:2099 propagates through proto_flow() and terminates nxc with a Rich
traceback, dropping back to shell. A honeypot on the network can silently kill an ongoing pentest scan with a confusing error that doesn't indicate hostile activity.

On the allowlist being too restrictive: valid NTLM hostnames follow NetBIOS naming rules (max 15 chars, restricted character set). Anything outside [a-zA-Z0-9_.-] in a hostname is already non-compliant. That said, I'm happy to widen it to cover edge cases like machine accounts (DC01$):

re.sub(r'[^a-zA-Z0-9_.$@ -]', '', self.hostname)

A denylist approach (stripping only ../, , \x00, \n) is riskier because it becomes a game of enumerating bad inputs -- encoding tricks, Windows reserved names (CON, NUL), alternate data streams, etc.

Happy to update the PR with the wider allowlist if that works for you.

[NeffIsBack]

On the DoS: the null byte crash actually kills the entire nxc process, not just the per-host worker. The ValueError: embedded null byte from open() at smb.py:2099 propagates through proto_flow() and terminates nxc with a Rich
traceback, dropping back to shell. A honeypot on the network can silently kill an ongoing pentest scan with a confusing error that doesn't indicate hostile activity.

Although I don't have the time at the moment to test and disprove this thesis, I highly doubt that this is the case. The entire proto flow is wrapped in a try&except block, catching every error that occurs while interacting with the target. The only way I see that you could escape that try&except block is to trigger an exception in pythons exception handling. Feel free to prove me otherwise by scanning a range of hosts with the rogue target killing the entire process.

On the allowlist being too restrictive: valid NTLM hostnames follow NetBIOS naming rules (max 15 chars, restricted character set). Anything outside [a-zA-Z0-9_.-] in a hostname is already non-compliant. That said, I'm happy to widen it to cover edge cases like machine accounts (DC01$):

re.sub(r'[^a-zA-Z0-9_.$@ -]', '', self.hostname)

A denylist approach (stripping only ../, , \x00, \n) is riskier because it becomes a game of enumerating bad inputs -- encoding tricks, Windows reserved names (CON, NUL), alternate data streams, etc.

Happy to update the PR with the wider allowlist if that works for you.

Since we have just eliminated the 15 chars limit by using the DNS hostname instead of the NetBIOS name in #1195, I am not sure if this restriction still applies. I wouldn't bet that there are SMB servers out there replying with a non-compliant DNS name containing weird characters.
Also, the hostname is always displayed so you would pretty much immediately see any weird control characters. It could be good idea tho to restrict hostnames to printable characters (should probably done in impacket) and then filter characters that could misplace files or similar in NetExec.

[TristanInSec]

Hi @NeffIsBack,

Fair point on the DoS -- it shouldn't crash with multiple targets since except Exception at connection.py:180 catches the ValueError and each target runs in its own thread. I updated the PR description.

(logger.exception() at line 187 prints the full traceback, which is what the screenshot shows and was misleading)

The path traversal is the main issue though, and that one doesn't raise any exception. A rogue NTLM hostname like ../../../payload flows straight into output_file_template (only : gets replaced at connection.py:247), so --sam/--lsa/--ntds/--dpapi write loot files outside ~/.nxc/logs/. Same thing for --generate-hosts-file with newlines -- arbitrary host entries get injected silently.

The sanitization here strips traversal sequences, null bytes, newlines, and non-printables from the hostname, which is basically what you suggested.

[NeffIsBack]

The path traversal is the main issue though, and that one doesn't raise any exception. A rogue NTLM hostname like ../../../payload flows straight into output_file_template (only : gets replaced at connection.py:247), so --sam/--lsa/--ntds/--dpapi write loot files outside ~/.nxc/logs/. Same thing for --generate-hosts-file with newlines -- arbitrary host entries get injected silently.

The sanitization here strips traversal sequences, null bytes, newlines, and non-printables from the hostname, which is basically what you suggested.

Yes, you can place log files such as the loot output outside of the intended folder and that bug should be fixed.

According to microsoft there are actually very restrictive rules for hostnames, so it would probably be fine to filter them prior to displaying them. However, we should alert the user if there is a difference between the received and the filtered output to alert the user for potential false positives (e.g. custom implementations that do not comply to the standard).

[TristanInSec]

Good idea. Added a warning log when the hostname gets sanitized, showing both the original and filtered value:

WARNING  Hostname contains invalid characters (received: '../../../payload'), sanitized to: '______payload'

This way operators notice if a server is sending non-compliant hostnames without silently changing what they see.