1 Topic by SheldonCooper (edited by SheldonCooper 2013-03-27 06:57:44)

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

Topic: Sniffer Application - suspected bug in Reassembly Process

Hey All,
I have noticed a rare crush in the sniffer Application. I debugged the Core file and reached a situation where the variable
sslBytes takes a negative value. Consequently, the following line in sniffer.c resulted in a crush:

in CheckPreRecord:
...

XMEMCPY(&ssl->buffers.inputBuffer.buffer[length],*sslFrame,*sslBytes)

it's clear why the code crushed here (sslBytes = -2928)
I started digging by looking at the values from the core file, following the entire path of the program, and got to the following line, where the I suspect the problem lies:
in sniffer.c, AdjustSequence(...):

if(reassemblyList){
   word32 newEnd = *expected + *sslBytes; //my calculations yielded newEnd = 4472
   if(newEnd > reassemblyList->begin){ //reassemblyList->begin = 1461 so this is true
      *sslBytes -= newEnd - reassemblyList->begin; //--> this yields sslBytes -= -2928
   }
}

why is the last line necessary? I couldn't understand, we already remove overlapping a few lines above...

for your convenience, here are the relevant values extracted from the core file:
packet = 0x7F696F4d28F6
ipInfo->length = 20
tcpInfo->length = 32
tcpInfo->sequence = 27041368
ipInfo->total = 299
length = 299
seqStart=session->srvSeqStart = 4225
expected = session->srvExpected = 1461, but you should use 4389 - see ** below
reassemblyList->begin = 1461
reassemblyList->end = 2868

i think these are all the values you need in order to trace the path of the program and reach teh same conclusion as I did...
**heads up:
in the function AdjustSequence, we modify the expected param by reference using sslBytes, which it helped compute. that means that in the core file we witness a value of $expected which is already influenced by the false calculation of sslBytes. since we add sslBytes (-2928) to expected after the calculation, I added 2928 to the value of $expected in the core file, which resulted in 4389. using this value, i was able to trace the entire program path to and get the value of sslBytes as it was seen in the core file.

if you need any more explanation of what I did in order to reach those conclusions, please let me know...


Dan

Share

  • From: Bozeman, MT
  • Registered: 2010-10-20
  • Posts: 647

Re: Sniffer Application - suspected bug in Reassembly Process

Hi Dan,

Can you explain more about how and why you modified the value of "expected" in AdjustSequence()?  I think this might be what's throwing your sslBytes calculations off, thus causing the crash.

If you don't mind me asking, what kind of project are you working on?  If you'd rather not post this info on the forums, feel free to email me directly at support (at) wolfssl (dot) com.

Thanks,
Chris

https://www.wolfssl.com

chrisc's Website

Share

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

Re: Sniffer Application - suspected bug in Reassembly Process

Hey Chris,
thanks for responding and sorry for my late reply.

Can you explain more about how and why you modified the value of "expected" in AdjustSequence()?  I think this might be what's throwing your sslBytes calculations off, thus causing the crash.

I didn't explain myself vividly enough. I am not modifying anything: I posted the values of the relevant variables as they were extracted from the dump (.core) file. Although the value of expected = session->srvExpected was extracted from the core file as being 1461, I specified that in order to truly reconstruct the path flow of the program, one should use expected = session->srvExpected = 4389. the reason for that being that the .core file gives the last "image" of the memory stack, and the value of session->srvExpected is modified (in the original code!) by adding to it the value of *sslBytes. since we are actually looking to trace back to the events that originally led to the value of sslBytes being negative, we should not use the values from core file effected by this undesired situation, but rather try to understand what were the values which lead to that situation. therefore, since session->srvExpected is modified AFTER sslBytes is obtained, the core file value for session->srvExpected is not relevant, we should try and conclude what was the value before hand (since it effects the flow of the program). due to the change being session->srvExpected += sslBytes, and sslBytes in the core file is -2928, I simply noted that one should use with session->srvExpected + 2928 in order to trace back what lead to sslBytes being negative.

to sum this: you can simply try and trace the code given the variable values I listed above with expected = session->srvExpected = 4389 and I am sure you will see the problem I encountered.

If you don't mind me asking, what kind of project are you working on?  If you'd rather not post this info on the forums, feel free to email me directly at support (at) wolfssl (dot) com.

I monitor my own server and prefer to do so passively (less intrusive to the application). the crushes I suffer occur rarely, but I imagine that since I use it continuously, even the less probable session values occur.

so, are you sure 

*sslBytes -= newEnd - reassemblyList->begin;

is necessary? it appears to me that even when removing it should still work (in fact, I tried it and it does work on my various examples). However, I am concerned that maybe it is necessary and I will now mess up a different session scenario.


thanks for your attention,


Dan

Share

  • From: Bozeman, MT
  • Registered: 2010-10-20
  • Posts: 647

Re: Sniffer Application - suspected bug in Reassembly Process

Dan,

Thanks for the explanation.  I see what you were doing now - just correcting the value of sslBytes to what it was prior to the core dump.

Let me explain a little bit about how AdjustSequence() works.  For this explanation, assume that we're expecting sequence 1,000, our incoming sequence is 800, with length = 700.  So, when we enter AdjustSequence():

expected = 1000
real = 800

if (real < *expected)

Here we test if the sequence we just got is before what we are currently expecting.  We may have already processed it (or some of it).

if (real + *sslBytes > *expected)

Here we catch the case where the sniffer has already consumed some of the front of the packet.  In this example case, the overlap would be 200 (1000 - 800).

*sslFrame += overlap;                                               
*sslBytes -= overlap;

As a result, we increment the frame pointer at offset 'real' by the overlap amount.  We also decrease the length (sslBytes) by the overlap.  This puts the frame pointer at 1,000 (the expected), and the sslBytes (length) at 500.

if (reassemblyList) {
     word32 newEnd = *expected + *sslBytes;
     if (newEnd > reassemblyList->begin)

Before just going ahead and processing bytes 1,000 - 1,500, we check to see whether the sniffer already has any of those bytes on the reassembly list.  Let's say for this example that the reassembly list already has bytes 1,200 - 1,500.

We detect this case by checking whether newEnd (1,500) is ahead of the beginning of the reassembly list.

*sslBytes -= newEnd - reassemblyList->begin;

If so, we remove the bytes we already have on the reassembly list.  In this example's case this would be:
500 -= 1500 - 1200, which is 200.  This means we have to process bytes 1,000 through 1,200 and then the reassembly list can take over.

if (newEnd > reassemblyList->end)

The sniffer also does a check to catch the case where it has gotten some bytes which are past the end of the reassembly list - thus not on it yet.  In this case, the end of the packet gets placed on the reassembly list.

Hopefully this helps clear things up a little.

- Chris

https://www.wolfssl.com

chrisc's Website

Share

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

Re: Sniffer Application - suspected bug in Reassembly Process

Hey Chris,
thanks again for your quick reply and sorry for my delayed response. I agree with the logic you presented, however the fact remains that the value I received for *sslBytes was negative. I suspect that "reassemblyList->begin" value is too large, I will therefore go to check the values there and investigate whether I have made any changes to the reassembly process which could have cause this, or maybe there is a bug. I am inclined to think reassemblyList->begin is "wrong" since the rest of the values reflect the latest incoming frame, and no changes are made there, while reassemblyList->begin depends on more logic. I will keep you posted if I come up with any conclusions...

thanks again,
Dan

Share