• Registered: 2011-05-31
  • Posts: 24

Topic: suspected bug in sniffer application + suggested fix

Hey all,
I got a seg fault while working on the sniffer application. If I am not mistaking, in the "CreateBuffer" function in sniffer.c, the:

assert(begin <= end)

might be wrong... I believe I have encountered a packet (I went through the core file to check the values) which has a sequence number of about (2^32-11) and sslBytes for that packet was about 213... resulting that the packet ends "at" 202. I suggest to add:

if(added < 0){
    added = (0xFFFFFFFFU - *begin) + end + 1
}

instead of asserting...

I will test and let you know if it worked. I suspect however this little change might cause other issues if not supported throughout the code...

let me know what you think,


regards,
DanC

Share

  • Registered: 2011-05-31
  • Posts: 24

Re: suspected bug in sniffer application + suggested fix

testing on my part shows that adding the mentioned code solves this particular seg fault which resulted in release mode (the assert wasn't active and thus a later memcpy used the *negative* $added value as param of type size_t to memcpy ---> seg fault).

1. i am still not sure of the +1 at the end of the expression there
2. while not yet sure, it might be that this fix alone is not complete, for now it appears to have solve this particular seg fault


DanC

Share

  • Registered: 2011-05-31
  • Posts: 24

Re: suspected bug in sniffer application + suggested fix

[Update]:
I am not sure about:

if(added < 0){
    added = (0xFFFFFFFFU - *begin) + end + 1
}

do we need the +1, maybe we need more? or less? I suspect this is not accurate... you ideas will be much appreciated..


DanC

Share

  • todd
  • Moderator
  • Offline
  • Registered: 2010-10-21
  • Posts: 98

Re: suspected bug in sniffer application + suggested fix

The wolfSSL embedded ssl sniffer adjusts packet sequences from actual to relative in a couple sports in sniffer.c, search for "rollover".  The sniffer packet buffer holds out of order packets until the full stream can be decoded and it uses relative sequences.  In order to hit the code you're talking about you've traced an SSL connection that sends more than 4GB of data in one direction?  That's potentially a misuse of SSL for a single connection, what's the use case if I may ask?

The "+ 1" is needed because a packet that starts at sequence 2 and goes through sequence 3 uses two bytes, not one.

end(3) - begin(2) + 1

I'm not sure if the packet buffer lists can handle more than 4GB of data, that will require some more testing on our end. 

Thanks for the report.

Share

  • Registered: 2011-05-31
  • Posts: 24

Re: suspected bug in sniffer application + suggested fix

Thanks for your reply.
I saw the "rollover" part in the code, I agree that a connection of more than 4Gb would cause the error I witnessed. However I am pretty sure that wasn't my case (it happened quite quickly after I run the application, no time for 4Gb to accumulate).
If I understand correctly - the rollover handling that takes place only addresses the situation where the current packet's sequence has rolled over and therefore is much smaller than the *expected sequence number* - in which case "real" is adjusted. however, what if the following scenario takes place:
the expected sequence number is close to 2^32, a packet with that SN arrives and this packets "ends" after 2^32. I believe a packet of this sort will raise the assert call in CreateBuffer, or worse (in Release mode), will cause a seg fault due to the later execution  of memcpy with a negative size_t value of [end-*begin] (which is negative, as the packet starts at around 2^32 and ends not far from 0).

does this make sense to you?

I hope I understood the code correctly.

thanks for your attention,

regards,
DanC

Share