xnih / satori

Python rewrite of passive OS fingerprinting tool
GNU General Public License v2.0
153 stars 25 forks source link

Potentially Redundant Signatures #13

Closed TheGZIP closed 3 years ago

TheGZIP commented 3 years ago

if we take a look at first condition in detectOddities,

https://github.com/xnih/satori/blob/f2f4d2330870f9965cebde9a9cc992c49172c486/satoriTCP.py#L200

All signatures ending with option E are supposed to contain 'P', yet there are 0 signatures containing this oddity. is this a code update issue?

xnih commented 3 years ago

Ok, up front, code is hacked together and based in part at least on P0Fv1, but doesn't mean it was well ported from c++ to delphi back in the day by me (2005 time frame?) and then from delphi to python.

But with that said lets take a look at it:

def tcpProcess(pkt, layer, ts, sExactList, saExactList, sPartialList, saPartialList): ... [tcpOpts, tcpTimeStampEchoReply, mss] = decodeTCPOptions(tcp1.opts) odd = detectOddities(ip4, ipHdrLen, ipVersion, tcpHdrLen, tcpFlags, tcp1, tcpOpts, tcpTimeStampEchoReply)

So tcpProcess grabs the tcpoptions from the packet and decodes them

def decodeTCPOptions(opts): ... for i in opts: if i.type == 0: res = res + 'E,' elif i.type == 1: res = res + 'N,' elif i.type == 2: mss = struct.unpack('!h',i.body_bytes)[0] res = res + 'M' + str(mss) + ',' elif i.type == 3: x = struct.unpack('!b',i.body_bytes)[0] res = res + 'W' + str(x) + ',' elif i.type == 4: res = res + 'S,'

Adding an E potentially to the data that is returned from decodeTCPOptions if i.type was equal to 0. We then send this to detectOddities.

def detectOddities(_ip, _ip_hlen, _ip_type, _tcp_hlen, _tcp_flags, _tcp, _tcp_options, _options_er): odd = '' if _tcp_options[:-1] == 'E': odd = odd + 'P' ...

and here check to see if the last character in the TCPOptions was an E, if so add a P (and potentially others)

tcpSignature = str(winSize) + ':' + str(ttl) + ':' + str(df) + ':' + str(ipHdrLen + tcpHdrLen) + ':' + tcpOpts + ':' + odd

So after all that, any signatures with ',E:' you're correct, none contain a P.

I looked back at some of p0f's old ones and some definitely had P's in it as expected and some didn't even when I think they should have at a quick glance.

S36:64:1:60:M1360,T,S,W0,E:.:SymbianOS:60xx

but others did like:

1024:64:0:60:W10,N,M265,T,E:P:-*NMAP:OS detection probe (1)

It's been "broken" like this for a LONG time (at least 2009 by quick glance at fingerprints), so I'm nervous of how many fingerprints I break by "fixing" it, but I'll definitely do some tests to see why it isn't working as designed because at a quick glance at the code I'm not seeing why it isn't working and evidently hasn't since I ported to python, but the fact that none of my tcp fingerprints have a P indicates its probably been broken since I first built it in delphi!

Thanks for finding a long standing bug and I'll see what I can do to fix it next time I'm doing code updates and what I'll probably do is duplicate fingerprints, adding the 'P' on any that "should" have it, but probably leave the old ones in there for anyone running old code and only updating fingerprints. I've got this fix and some other stuff to do one of these days when I have time to sit down and code again.

xnih commented 3 years ago

implemented the fix, but still have to find and update all signatures next week, then I'll close this out.

xnih commented 3 years ago

Updated all the fingerprints that had ',:e:' and updated to put the 'p' in there and removed the '.' if that was there since it shouldn't have been had the fingerprints been working correctly.

Thanks for finding this, even if it was a pain to update that many fingerprints that have been broken since I wrote the original version! :)