-
Notifications
You must be signed in to change notification settings - Fork 2
Description
I was reading through smb.py and noticed several things that appeared different from the impacket libraries. I had Claude help me analyze the code to see if my thoughts were correct and make sure to reference the official MS specs. The following was written with the help of AI but was human reviewed.
Bugs 1, 2, 3, 6, and 7 directly block the authentication path for whole categories of clients. Bugs 4, 5, 8, and 9 introduce wire-level malformations that can cause strict clients to drop the connection before reaching SESSION_SETUP.
Bug 1: SMB2_DIALECTS maps "SMB 2.???" to 0x0000 instead of 0x02FF
Spec reference: [MS-SMB2] §2.2.4 SMB2 NEGOTIATE Response (p. 56): the DialectRevision field table defines 0x02FF as the SMB2 wildcard revision number, sent only in response to a multi-protocol negotiate request containing the "SMB 2.???" dialect string.
Spec reference: [MS-SMB2] §3.3.5.3.1 SMB 2.1 or SMB 3.x Support (p. 302): "DialectRevision MUST be set to 0x02FF."
The constant 0x0000 is the numeric value of the SMB2_NEGOTIATE command opcode, not a dialect revision. Writing it into DialectRevision sends a semantically invalid response to any client that offered "SMB 2.???" in its SMBv1 multi-protocol negotiate, which is every modern Windows and Samba client.
# Current (wrong) -- 0x0000 is the SMB2_NEGOTIATE command opcode, not a dialect revision
SMB2_DIALECTS = {
# ...
0x0000: "SMB 2.???",
}# Fixed
SMB2_DIALECTS = {
smb2.SMB2_DIALECT_002: "SMB 2.002",
smb2.SMB2_DIALECT_21: "SMB 2.1",
smb2.SMB2_DIALECT_30: "SMB 3.0",
smb2.SMB2_DIALECT_302: "SMB 3.0.2",
smb2.SMB2_DIALECT_311: "SMB 3.1.1",
# Wildcard revision per [MS-SMB2] §2.2.4 (p. 56) and §3.3.5.3.1 (p. 302).
# Sent only in response to a multi-protocol negotiate containing "SMB 2.???".
# Signals the client to send a follow-up SMB2_NEGOTIATE to select the actual dialect.
# This is a signalling value only -- it MUST NOT be the final negotiated dialect.
smb2.SMB2_DIALECT_WILDCARD: "SMB 2.???", # 0x02FF
}
# Dialects the server will actually negotiate; excludes the wildcard signalling value.
_SMB2_NEGOTIABLE_DIALECTS: frozenset[int] = frozenset(SMB2_DIALECTS) - {
smb2.SMB2_DIALECT_WILDCARD
}Tracing how SMB2_DIALECT_REV is built and used, because the wrong constant in SMB2_DIALECTS has a cascading effect that is not immediately obvious.
SMB2_DIALECT_REV is constructed by inverting SMB2_DIALECTS:
SMB2_DIALECT_REV = {v: k for k, v in SMB2_DIALECTS.items()}
# With the current (buggy) SMB2_DIALECTS this produces:
# {
# "SMB 2.002": 0x0202,
# "SMB 2.1": 0x0210,
# "SMB 3.0": 0x0300,
# "SMB 3.0.2": 0x0302,
# "SMB 3.1.1": 0x0311,
# "SMB 2.???": 0x0000, # WRONG: 0x0000 is the SMB2_NEGOTIATE command opcode
# }SMB2_DIALECT_REV is used in exactly three places in smb1_negotiate_protocol:
# Callsite 1 (line 366) - Python "in" check on dict KEYS only; value is never read
if dialect in SMB2_DIALECT_REV and handler.smb_config.smb2_support:
# Callsite 2 (line 371) - Python "in" check on dict KEYS only; value is never read
if target_dialect in SMB2_DIALECT_REV:
# Callsite 3 (line 373) - VALUE lookup; this is where the bug fires
command = smb2_negotiate(handler, SMB2_DIALECT_REV[target_dialect])Python's in operator on a dict checks whether a string exists as a key. The numeric value on the other side is never touched during a membership test. Callsites 1 and 2 correctly recognise "SMB 2.???" as an SMB2 dialect string regardless of whether the value is 0x0000 or 0x02FF. Neither callsite ever needs to change.
Callsite 3 is a value lookup. When target_dialect == "SMB 2.???" this fetches the number and passes it directly to smb2_negotiate() as target_revision, which writes it into command["DialectRevision"] in the wire response. With 0x0000 the client receives the SMB2_NEGOTIATE command opcode in the DialectRevision field, which it cannot interpret as a valid dialect response, and the connection fails before authentication.
All three existing callsites work without modification once the value is correct.
Bug 2: smb2_negotiate_protocol selects an arbitrary dialect and the None guard is dead code
Spec reference: [MS-SMB2] §3.3.5.4 Receiving an SMB2 NEGOTIATE Request (p. 304): "The server MUST select the greatest common dialect between the dialects it implements and the Dialects array of the SMB2 NEGOTIATE request."
Spec reference: [MS-SMB2] §3.3.5.4 (p. 304): "If the DialectCount of the SMB2 NEGOTIATE Request is 0, the server MUST fail the request with STATUS_INVALID_PARAMETER."
The current code takes req_dialects[-1], which is whatever the client happened to put last in its array. This is not the greatest common dialect. It may also select the wildcard 0x02FF, which [MS-SMB2] §2.2.4 (p. 56) defines as a signalling value that "MUST NOT" appear as the final negotiated DialectRevision.
The if dialect is None guard immediately below is also dead code. In Python, list[-1] raises IndexError on an empty list — it never returns None. The guard must instead be an empty-list check placed before the assignment.
# Current (wrong) -- REVISIT comment acknowledges this is incomplete
dialect = req_dialects[-1]
if dialect is None: # can never be True; list[-1] raises IndexError on empty list
...# Fixed: guard empty list per [MS-SMB2] §3.3.5.4, then select greatest common dialect
if not req_dialects:
handler.logger.fail("<SMB2_NEGOTIATE> Client sent no dialects")
raise BaseProtoHandler.TerminateConnection
dialect = max(
(d for d in req_dialects if d in _SMB2_NEGOTIABLE_DIALECTS),
default=None,
)
if dialect is None:
handler.logger.fail(
f"<SMB2_NEGOTIATE> No mutually supported dialect in client offer: "
f"{str_req_dialects}"
)
raise BaseProtoHandler.TerminateConnectionBug 3: smb1_negotiate_protocol breaks on the first SMB2 dialect found, never reaching the wildcard
Spec reference: [MS-SMB2] §3.3.5.3.1 SMB 2.1 or SMB 3.x Support (p. 301–302): "the server MUST scan the dialects provided for the dialect string 'SMB 2.???'. If the string is not present, continue to section 3.3.5.3.2. If the string is present, the server MUST respond with an SMB2 NEGOTIATE Response... DialectRevision MUST be set to 0x02FF."
The current loop uses break at the first SMB2 dialect found. Because clients list dialects in ascending version order, the first SMB2 entry is always "SMB 2.002". The loop therefore never reaches "SMB 2.???", which Windows places at the end of its dialect list. The spec is unambiguous: presence of "SMB 2.???" is the primary condition to check, and when it is present the response must use the wildcard revision. The existing code skips this entirely.
A secondary problem follows: the SMBv1 fallback path (index = 0) is supposed to be reached only when the client offers no SMB2 dialects at all. Because break fires on the first SMB2 entry, the SMBv1 branch is structurally unreachable for any client that lists even one SMB2 dialect alongside its SMBv1 dialects.
# Current (wrong) -- breaks at "SMB 2.002", never scans for "SMB 2.???"
index = 0
for i, dialect in enumerate(dialects):
if dialect in SMB2_DIALECT_REV and handler.smb_config.smb2_support:
index = i
break # stops at the first (lowest) SMB2 entry
target_dialect = dialects[index]The correct priority, per [MS-SMB2] §3.3.5.3.1 followed by §3.3.5.3.2 (p. 301–303), is: scan for "SMB 2.???" first; if not present fall back to the highest explicit SMB2 dialect offered; if no SMB2 dialects are offered, select the highest SMBv1 dialect.
# Fixed: scan per [MS-SMB2] §3.3.5.3.1 -- prefer wildcard, then highest SMB2, then SMBv1
smb2_entries = [
(i, d) for i, d in enumerate(dialects)
if d in SMB2_DIALECT_REV and handler.smb_config.smb2_support
]
if smb2_entries:
# Per §3.3.5.3.1: if "SMB 2.???" is present the response MUST use DialectRevision 0x02FF.
# Fall back to the last entry (highest explicit dialect) if the wildcard is not offered.
wildcard_entry = next(
((i, d) for i, d in smb2_entries if d == "SMB 2.???"), None
)
index, target_dialect = wildcard_entry or smb2_entries[-1]
else:
# No SMB2 dialects offered, or SMB2 disabled -- continue to SMBv1 path.
index, target_dialect = 0, dialects[0]Bug 4: SMB3_get_target_capabilities negotiate context loop never advances past the first context
Spec reference: [MS-SMB2] §2.2.3.1 SMB2 NEGOTIATE_CONTEXT Request Values (p. 49): "Subsequent negotiate contexts MUST appear at the first 8-byte-aligned offset following the previous negotiate context."
The variable offset is correctly recalculated on each loop iteration, but it is never applied to raw_context_list. Every call to smb3.SMB2NegotiateContext(raw_context_list) starts at byte zero regardless of which iteration the loop is on. The result is that the first context is parsed N times and all subsequent contexts are silently ignored. For an SMB 3.1.1 client that sends SMB2_PREAUTH_INTEGRITY_CAPABILITIES first, then SMB2_ENCRYPTION_CAPABILITIES, then SMB2_SIGNING_CAPABILITIES, the server never processes the client's encryption or signing preferences. This can cause strict clients to reject the connection when the server's response does not reflect capabilities the client knows it advertised.
# Current (wrong) -- offset calculated but never sliced into the buffer
offset = 0
for _ in range(context_data["NegotiateContextCount"]):
context = smb3.SMB2NegotiateContext(raw_context_list) # always offset 0
match context["ContextType"]:
# ...
offset += context["DataLength"] + 8
offset += (8 - (offset % 8)) % 8# Fixed -- slice raw_context_list at each offset per [MS-SMB2] §2.2.3.1 (p. 49)
offset = 0
for _ in range(context_data["NegotiateContextCount"]):
context = smb3.SMB2NegotiateContext(raw_context_list[offset:]) # advance the window
match context["ContextType"]:
# ...
offset += context["DataLength"] + 8
offset += (8 - (offset % 8)) % 8Bug 5: smb2_negotiate hardcodes +2 padding and fills it with \xff\xff for SMB 3.1.1
Spec reference: [MS-SMB2] §2.2.4 SMB2 NEGOTIATE Response (p. 57): NegotiateContextOffset specifies "the offset, in bytes, from the beginning of the SMB2 header to the first 8-byte aligned negotiate context in NegotiateContextList."
Spec reference: [MS-SMB2] §2.2.4 (p. 57): "Padding (variable): Optional padding between the end of the Buffer field and the first negotiate context in the NegotiateContextList so that the first negotiate context is 8-byte aligned."
The SecurityBufferOffset is fixed at 0x80 (128 bytes from the SMB2 header start). The NegotiateContextList must therefore begin at the first 8-byte-aligned offset after 0x80 + SecurityBufferLength. Since 128 is already a multiple of 8, the required padding length is (8 - (SecurityBufferLength % 8)) % 8. The hardcoded +2 is only correct when SecurityBufferLength % 8 == 6. For any other blob length the NegotiateContextOffset points into the middle of the padding rather than the start of the first context, and the context list is misaligned.
The padding bytes are also written as b"\xff\xff". The spec instructs that any fields not explicitly assigned a value MUST be set to 0 ([MS-SMB2] §2.2.1 general header rules; general Reserved field convention throughout the spec). Using \xff violates this and may cause strict client-side validation to fail.
# Current (wrong) -- hardcoded +2 padding, wrong padding byte value
command["NegotiateContextOffset"] = 0x80 + command["SecurityBufferLength"] + 2
command["NegotiateContextList"] = b"\xff\xff" + context_data# Fixed -- compute 8-byte alignment dynamically per [MS-SMB2] §2.2.4 (p. 57)
sec_buf_end = 0x80 + command["SecurityBufferLength"]
pad_len = (8 - (sec_buf_end % 8)) % 8
command["NegotiateContextOffset"] = sec_buf_end + pad_len
command["NegotiateContextList"] = b"\x00" * pad_len + context_dataBug 6: smb1_session_setup silently drops credentials from all legacy clients (WordCount=13)
This is the highest-priority bug for credential capture.
Spec reference: [MS-SMB] §2.2.4.6.1 SMB_COM_SESSION_SETUP_ANDX Client Request Extensions (p. 53–54): "When extended security is being used... the request MUST take the following form... WordCount (1 byte): The value of this field MUST be 0x0C."
The spec defines two distinct SMB_COM_SESSION_SETUP_ANDX request formats. WordCount=0x0C (12) is the extended security format: the client sends a SPNEGO/NTLMSSP blob through the full token negotiation dance, and it is what every client from Windows Vista onwards uses. WordCount=0x0D (13) is the legacy non-extended security format defined in [MS-CIFS] §2.2.4.53.1: the client places raw LM and NTLM hash responses directly in AnsiPwd and UnicodePwd fields with no SPNEGO wrapping at all. Windows 9x, Windows NT4, and a very large number of embedded devices including NAS boxes, printers, and IP cameras use this path exclusively because they were built before extended security was standardised.
The current code silently returns without sending any response or logging anything when WordCount != 12. From this tool's perspective this is the worst possible outcome: these clients arrive presenting credentials directly in their first packet and those credentials are silently discarded. The client then hangs indefinitely waiting for a server response that never comes.
# Current (wrong) -- WordCount=13 clients silently hang; credentials are never captured
if command["WordCount"] == 12:
# ... extended security path ...
handler.send_smb1_command(...)
# else: nothing -- client receives no response, hangs, credentials discarded# Fixed -- handle both paths per [MS-SMB] §2.2.4.6.1 (p. 53-54)
if command["WordCount"] == 12:
# Extended security path (SPNEGO/NTLMSSP blob) -- Windows Vista+, macOS, modern Samba
setup_params = smb.SMBSessionSetupAndX_Extended_Parameters(command["Parameters"])
setup_data = smb.SMBSessionSetupAndX_Extended_Data()
setup_data["SecurityBlobLength"] = setup_params["SecurityBlobLength"]
setup_data.fromString(command["Data"])
resp_token, error_code = handler.authenticate(setup_data["SecurityBlob"])
parameters = smb.SMBSessionSetupAndX_Extended_Response_Parameters()
data = smb.SMBSessionSetupAndX_Extended_Response_Data(flags=packet["Flags2"])
data["SecurityBlob"] = resp_token
data["SecurityBlobLength"] = len(resp_token)
parameters["SecurityBlobLength"] = len(resp_token)
data["NativeOS"] = smbserver.encodeSMBString(
packet["Flags2"], handler.smb_config.smb_server_os
)
data["NativeLanMan"] = smbserver.encodeSMBString(
packet["Flags2"], handler.smb_config.smb_server_os
)
handler.send_smb1_command(
smb.SMB.SMB_COM_SESSION_SETUP_ANDX, data, parameters, packet,
error_code=error_code,
)
elif command["WordCount"] == 13:
# Legacy non-extended path (raw LM/NTLM hashes) -- Windows 9x/NT4/embedded devices.
# Per [MS-CIFS] §2.2.4.53.1: AnsiPwd contains the LM response,
# UnicodePwd contains the NTLM response, both computed against the server challenge
# sent in the negotiate response SessionKey field.
setup_params = smb.SMBSessionSetupAndX_Parameters(command["Parameters"])
setup_data = smb.SMBSessionSetupAndX_Data()
setup_data["AnsiPwdLength"] = setup_params["AnsiPwdLength"]
setup_data["UnicodePwdLength"] = setup_params["UnicodePwdLength"]
setup_data.fromString(command["Data"])
# The raw LM/NTLM hash responses are directly usable credentials.
NTLM_report_legacy_auth(
username=setup_data["Account"],
domain=setup_data["PrimaryDomain"],
lm_response=setup_data["AnsiPwd"],
nt_response=setup_data["UnicodePwd"],
client=handler.client_address,
session=handler.config,
logger=handler.logger,
)
parameters = smb.SMBSessionSetupAndXResponse_Parameters()
data = smb.SMBSessionSetupAndXResponse_Data(flags=packet["Flags2"])
data["NativeOS"] = smbserver.encodeSMBString(
packet["Flags2"], handler.smb_config.smb_server_os
)
data["NativeLanMan"] = smbserver.encodeSMBString(
packet["Flags2"], handler.smb_config.smb_server_os
)
data["PrimaryDomain"] = smbserver.encodeSMBString(
packet["Flags2"], handler.smb_config.smb_fqdn
)
handler.send_smb1_command(
smb.SMB.SMB_COM_SESSION_SETUP_ANDX, data, parameters, packet,
error_code=handler.smb_config.smb_error_code,
)
else:
handler.logger.fail(
f"<SMB_COM_SESSION_SETUP_ANDX> Unrecognised WordCount: {command['WordCount']}"
)
raise BaseProtoHandler.TerminateConnectionNTLM_report_legacy_auth must be implemented in dementor.protocols.ntlm. The LM response in AnsiPwd and the NTLM response in UnicodePwd are computed against the 8-byte server challenge sent in _dialects_parameters["SessionKey"] during the negotiate phase. That challenge value must be tracked in handler state for the response to be verifiable.
Bug 7: smb1_negotiate_protocol crashes with IndexError on an empty dialect list
Spec reference: [MS-SMB2] §3.3.5.4 (p. 304): "If the DialectCount of the SMB2 NEGOTIATE Request is 0, the server MUST fail the request with STATUS_INVALID_PARAMETER." The equivalent robustness requirement applies to the SMBv1 path.
If a client sends SMB_COM_NEGOTIATE with no dialect strings, req["Data"].split(b"\x02")[1:] produces an empty list. The dialect selection logic from the Bug 3 fix then falls to its else branch and attempts dialects[0], raising IndexError. The connection dies with a Python traceback rather than a clean logged error.
# Current (wrong) -- no guard; IndexError crashes the handler on an empty dialect list
dialects = [
dialect.rstrip(b"\x00").decode(errors="replace")
for dialect in req["Data"].split(b"\x02")[1:]
]
# If dialects == [], subsequent access to dialects[0] raises IndexError# Fixed -- guard immediately after parsing
dialects = [
dialect.rstrip(b"\x00").decode(errors="replace")
for dialect in req["Data"].split(b"\x02")[1:]
]
if not dialects:
handler.logger.fail("<SMB_COM_NEGOTIATE> Client sent no dialect strings")
raise BaseProtoHandler.TerminateConnectionBug 8: authenticate NTLM token length guard is off by one
The NTLM MessageType field sits at byte offset 8 within the token, meaning a valid read of token[8] requires len(token) >= 9. The guard if len(token) < 8 permits a token of exactly 8 bytes, which passes the check but then raises IndexError on the very next line when token[8] is accessed. Any client that sends a malformed 8-byte NTLM fragment hits an unhandled exception rather than the clean logged failure the guard is intended to produce.
# Current (wrong) -- guard permits len==8, which crashes on token[8]
if len(token) < 8:
handler.logger.fail(...)
raise BaseProtoHandler.TerminateConnection
match token[8]: # IndexError if len(token) == 8# Fixed -- the MessageType field at offset 8 requires at least 9 bytes
if len(token) < 9:
handler.logger.fail(
f"<{command_name}> NTLM token too short to contain MessageType: {len(token)} bytes"
)
raise BaseProtoHandler.TerminateConnectionBug 9: SMB3_get_neg_context_pad returns \xff padding bytes between negotiate contexts
Spec reference: [MS-SMB2] §2.2.3.1 (p. 49): "Subsequent negotiate contexts MUST appear at the first 8-byte-aligned offset following the previous negotiate context." While the spec does not enumerate the padding byte value explicitly, the general rule throughout [MS-SMB2] §2.2.1 and every Reserved field definition in the spec is that fields not carrying data MUST be set to 0. Using \xff is non-standard and may be rejected by strict client-side validation. This is a separate instance of the same wrong-padding-byte problem as Bug 5, but at a different level of the structure — Bug 5 concerns the gap between the security buffer and the NegotiateContextList, while this bug concerns the gap between individual contexts within that list. Both locations must be zero-filled.
# Current (wrong) -- padding between contexts uses 0xff
def SMB3_get_neg_context_pad(data_len: int) -> bytes:
return b"\xff" * ((8 - (data_len % 8)) % 8)# Fixed -- padding bytes set to 0x00, consistent with [MS-SMB2] §2.2.3.1 (p. 49)
def SMB3_get_neg_context_pad(data_len: int) -> bytes:
return b"\x00" * ((8 - (data_len % 8)) % 8)Combined impact table
| # | Location | Spec reference | Root cause | Effect on auth capture |
|---|---|---|---|---|
| 1 | SMB2_DIALECTS |
[MS-SMB2] §2.2.4 (p. 56), §3.3.5.3.1 (p. 302) | 0x0000 instead of 0x02FF for wildcard |
Client receives corrupt DialectRevision; cannot reach SESSION_SETUP |
| 2 | smb2_negotiate_protocol |
[MS-SMB2] §3.3.5.4 (p. 304) | req_dialects[-1] is not the greatest common dialect; dead None guard |
May negotiate an unsupported or wildcard dialect; empty list crashes before auth |
| 3 | smb1_negotiate_protocol |
[MS-SMB2] §3.3.5.3.1 (p. 301–302) | Loop stops at the first (lowest) SMB2 dialect | Wildcard upgrade path never triggered; two-step negotiation never initiated |
| 4 | SMB3_get_target_capabilities |
[MS-SMB2] §2.2.3.1 (p. 49) | offset computed but never applied to slice |
SMB 3.1.1 cipher/signing contexts silently ignored; strict clients may drop connection |
| 5 | smb2_negotiate |
[MS-SMB2] §2.2.4 (p. 57) | Hardcoded +2 and b"\xff\xff" padding |
NegotiateContextOffset misaligned for most blob lengths; invalid padding bytes |
| 6 | smb1_session_setup |
[MS-SMB] §2.2.4.6.1 (p. 53–54) | WordCount=13 path unhandled |
Windows 9x / NT4 / embedded device credentials silently discarded |
| 7 | smb1_negotiate_protocol |
[MS-SMB2] §3.3.5.4 (p. 304) | No guard on empty dialects list |
IndexError crash on malformed negotiate; connection dies without logging |
| 8 | authenticate |
NTLM message structure | len(token) < 8 should be < 9 |
IndexError on exactly 8-byte token; unhandled exception instead of logged failure |
| 9 | SMB3_get_neg_context_pad |
[MS-SMB2] §2.2.3.1 (p. 49) | Returns b"\xff" between negotiate contexts |
Non-zero inter-context padding bytes; may cause strict SMB 3.1.1 client validation to fail |