SMB_FIND_FILE_BOTH_DIRECTORY_INFO FileName non-NUL terminated

[GoogleCodeExporter]

The SMBFindFileBothDirectoryInfo structure implements UnicodeStructure, which 
expects the unicode strings to be double NUL terminated.

However, the SMB_FIND_FILE_BOTH_DIRECTORY_INFO uses the FileNameLength 
parameter to determine bounds of FileName and not double NUL.

This means that SMBFindFileBothDirectoryInfo is unable to unpack the data 
correctly.

This appears to be the same for all FIND information levels.

/usr/local/lib/python2.7/dist-packages/impacket/smb.pyc in __init__(self, 
flags, **kargs)
    845         else:
    846             self.structure = self.AsciiStructure
--> 847         return Structure.__init__(self, **kargs)
    848 
    849 class SMBCommand_Parameters(Structure):

/usr/local/lib/python2.7/dist-packages/impacket/structure.pyc in __init__(self, 
data, alignment)
     82         self.rawData   = data
     83         if data is not None:
---> 84             self.fromString(data)
     85         else:
     86             self.data = None

/usr/local/lib/python2.7/dist-packages/impacket/structure.pyc in 
fromString(self, data)
    147                 dataClassOrCode = field[2]
    148             try:
--> 149                 self[field[0]] = self.unpack(field[1], data[:size], 
dataClassOrCode = dataClassOrCode, field = field[0])
    150             except Exception,e:
    151                 e.args += ("When unpacking field '%s | %s | %r[:%d]'" % (field[0], field[1], data, size),)

/usr/local/lib/python2.7/dist-packages/impacket/structure.pyc in unpack(self, 
format, data, dataClassOrCode, field)
    343         if format == 'u':
    344             if data[-2:] != '\x00\x00':
--> 345                 raise Exception, ("%s 'u' field is not NUL-NUL 
terminated: %r" % (field, data))
    346             return data[:-2] # remove trailing NUL
    347 

Original issue reported on code.google.com by carlruth...@gmail.com on 24 Aug 2014 at 10:57

[GoogleCodeExporter]

Looks like this is an issue with the documentation of SMB_STRING and not 
implementing it correctly.

This has been raised with Microsoft previously:
1. According to 2.2.1.1, an SMB_STRING is a "null-terminated character sequence 
fields that may be encoded in either Unicode or OEM characters".
However, according to the document notes, the FileName field in:
SMB_INFO_QUERY_EA_SIZE,
SMB_INFO_QUERY_EAS_FROM_LIST,
SMB_FIND_FILE_DIRECTORY_INFO
SMB_FIND_FILE_FULL_DIRECTORY_INFO
SMB_FIND_FILE_NAMES_INFO
SMB_FIND_FILE_BOTH_DIRECTORY_INFO,

which is denoted as SMB_STRING, does not terminate in 2 null bytes.
There is clearly a logical error here.

http://social.msdn.microsoft.com/Forums/en-US/713b6ce7-183b-489c-8b97-dbcda8ce7a
46/mscifs-possible-documentaion-mistakes?forum=os_specifications

Original comment by carlruth...@gmail.com on 24 Aug 2014 at 11:14

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Hey mate.. thanks for the bug report + research!

I'll take a look at it.. how did you find this?.. coding a client asking for 
those commands or through the smbserver.py?

If you have a repro test code that'd be great...

Original comment by bet...@gmail.com on 24 Aug 2014 at 2:23

• Changed state: Accepted
• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

I've been working on some SMB pcap parsing, which means understanding both 
ends. Fortunately, the smbserver provides significant help with at least one 
direction. I've used wireshark as my source of truth for the data layout.

I've attached an example of the FileName without double NUL.

The padding aligning can make it seem like there is a NULs, so it took a few 
test captures to get it.

Original comment by carlruth...@gmail.com on 24 Aug 2014 at 3:13

• Added labels: ****
• Removed labels: ****

Attachments:

• testcase.pcap

[GoogleCodeExporter]

Thanks for the pcap file! I'll definitely take a look it at.

We all love wireshark :).. but don't forget about the Microsoft Network Monitor 
(http://www.microsoft.com/en-us/download/details.aspx?id=4865). With all the 
Windows protocol parsers on, it's just awesome. Might be useful for you. It was 
key for me to develop the DCOM runtime, wireshark is not good parsing that 
part..

Original comment by bet...@gmail.com on 24 Aug 2014 at 3:28

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Thanks for the awesome tip! And yeah, I can see what you mean. Very 
comprehensive and having quick access to the type definitions, plus 
calculations for offsets. This will definitely save me some time.

Original comment by carlruth...@gmail.com on 24 Aug 2014 at 3:42

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Good to know mate..

Hey.. take a look at this smb.py and tell me how it goes. Keep in mind you will 
need to manually utf-16 decode the contents of the FileName structure member.

In case you didn't know, inside your code you can easily see how the structures 
are being parsed by calling the dump() method (after you did a fromString() or 
created an instance of a structure and fed some data).

I'm assuming you know where to put this smb.py and be sure it is loaded by your 
code (e.g. being in the PYTHONPATH). If you have problems with this let me know

tell me how it goes..

Original comment by bet...@gmail.com on 24 Aug 2014 at 9:11

• Added labels: ****
• Removed labels: ****

Attachments:

• smb.py

[GoogleCodeExporter]

Perfect. That works a treat!

Cheers for this. After seeing your changes, it makes a lot on sense. Very 
simple.

Nice work on this project :)

Original comment by carlruth...@gmail.com on 30 Aug 2014 at 5:29

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Great to know it works ;).. I just updated trunk with these changes.. 
https://code.google.com/p/impacket/source/detail?r=1259

I'm glad you like impacket! :).. Let me know if you have other questions..

closing this ticket.

cheers
beto

Original comment by bet...@gmail.com on 30 Aug 2014 at 8:22

• Changed state: Fixed
• Added labels: ****
• Removed labels: ****