-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Open
Description
Hi, while writing test coverage for our own fork of impacket i have found this and some other bugs
Configuration
- impacket version: 0.14.0.dev (from
impacket-master.zip) , also reproducible on 0.13.0 - Python version: 3.11.2
- Target OS: Linux x86_64
Debug Output With Command String
Command:
python -c "import sys,faulthandler; sys.path.insert(0,'./'); faulthandler.enable(); faulthandler.dump_traceback_later(1.0, repeat=False, exit=True); from impacket.ntlm import AV_PAIRS, NTLMSSP_AV_DOMAINNAME; av=AV_PAIRS(); print('Starting membership test...', flush=True); print(NTLMSSP_AV_DOMAINNAME in av, flush=True)"Output:
Starting membership test...
Timeout (0:00:01)!
Thread 0x00007eacbf53a180 (most recent call first):
File "/.../impacket/ntlm.py", line 229 in __getitem__
File "<string>", line 1 in <module>
PCAP
N/A
Additional context
What happens: key in av_pairs can hang forever.
Why: AV_PAIRS implements __getitem__ but not __contains__ or __iter__. For in, Python can fall back to the sequence protocol and call __getitem__(0), __getitem__(1), __getitem__(2), … until IndexError. But AV_PAIRS.__getitem__ never raises IndexError (it returns None), so the membership test can become an unbounded loop.
Location: impacket/ntlm.py class AV_PAIRS around __getitem__.
Suggested fixes:
- Implement
__contains__to checkkey in self.fields - Implement
__iter__(or at least make sequence fallback impossible) - Optional: change
__getitem__to raiseKeyErrorfor missing dict keys (and/or raiseIndexErrorfor integer indices) if you want to prevent sequence semantics entirely.
Example patch:
def __contains__(self, key):
return key in self.fields
def __iter__(self):
return iter(self.fields)Metadata
Metadata
Assignees
Labels
No labels