Skip to content

feat(ntlm): setting wrong values for Target{Info,Name}*#4973

Closed
p-l- wants to merge 1 commit intosecdev:masterfrom
p-l-:fix-ntlmssp-fake-values
Closed

feat(ntlm): setting wrong values for Target{Info,Name}*#4973
p-l- wants to merge 1 commit intosecdev:masterfrom
p-l-:fix-ntlmssp-fake-values

Conversation

@p-l-
Copy link
Copy Markdown
Member

@p-l- p-l- commented Apr 18, 2026

Checklist:

  • I squashed commits belonging together
  • I added unit tests or explained why they are not relevant
  • I executed the regression tests (using tox)
  • If the PR is still not finished, please create a Draft Pull Request

This allows setting fake values for Target{Name,Info}* in NTLM.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.31%. Comparing base (5009233) to head (3dcd937).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4973   +/-   ##
=======================================
  Coverage   80.31%   80.31%           
=======================================
  Files         381      381           
  Lines       93630    93640   +10     
=======================================
+ Hits        75202    75211    +9     
- Misses      18428    18429    +1     
Files with missing lines Coverage Δ
scapy/layers/ntlm.py 81.70% <100.00%> (+0.38%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends Scapy’s NTLM server-side token customization (NTLM_VALUES) to allow spoofing Target{Name,Info}{Len,MaxLen,BufferOffset} in NTLM CHALLENGE messages via byte-level patching, avoiding payload-layout-driven padding/allocations.

Changes:

  • Add server-side byte-patching for TargetName* and TargetInfo* security-buffer header fields in NTLM CHALLENGE serialization.
  • Add unit tests validating correct patch offsets, no excessive allocation, and survival through SPNEGO/ASN.1 re-encoding.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
scapy/layers/ntlm.py Implements byte-level patching of CHALLENGE header fields when NTLM_VALUES requests spoofed TargetName* / TargetInfo* values.
test/scapy/layers/ntlm.uts Adds regression tests ensuring patched bytes are present at correct offsets and do not trigger large allocations, including via SPNEGO wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scapy/layers/ntlm.py
Comment on lines +1921 to 1939
if _byte_patches:
# Serialize now, patch the bytes, then replace tok with a
# thin wrapper whose bytes() returns the patched bytes
# directly — this survives re-encoding in SPNEGO/ASN.1.
_tok_bytes = bytearray(bytes(tok))
for off, fmt, val in _byte_patches:
sz = struct.calcsize(fmt)
_tok_bytes[off : off + sz] = struct.pack(fmt, val)

class _PatchedTok(object):
"""Wrapper: bytes() returns pre-built patched NTLM bytes."""

def __bytes__(self_inner):
return bytes(_tok_bytes)

tok = _PatchedTok()

# Store for next step
Context.chall_tok = tok
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

When _byte_patches is non-empty, tok is replaced by _PatchedTok and then stored in Context.chall_tok. This breaks the subsequent server-side authentication path because _checkLogin() (and NTLMSSP_DOMAIN._getSessionBaseKey()) later access Context.chall_tok.ServerChallenge, NegotiateFlags, etc., which _PatchedTok does not provide. Keep Context.chall_tok as the real NTLM_CHALLENGE packet (e.g., store the original tok in the context and only wrap/patch the returned token), or implement a proxy that delegates attribute access to the underlying packet while overriding __bytes__.

Copilot uses AI. Check for mistakes.
Comment thread scapy/layers/ntlm.py
Comment on lines +1927 to +1928
sz = struct.calcsize(fmt)
_tok_bytes[off : off + sz] = struct.pack(fmt, val)

This comment was marked as resolved.

@p-l- p-l- requested a review from gpotter2 April 18, 2026 22:20
@p-l- p-l- closed this Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants