google / binexport

Export disassemblies into Protocol Buffers
Apache License 2.0
1.05k stars 204 forks source link

Duplicate BB export error #4

Closed togacure closed 7 years ago

togacure commented 8 years ago

One function in module primary_deleteall Second (equ) function in module after rebuild: secondary_deleteall Why it is possible (duplicate BB)? Why unconditional jmp split with another BB-body? Analize result both functions in IDA is equ.

cblichmann commented 8 years ago

Yes, this behavior is confusing. However, I lack a bit of context. Is this from CVE-2014-4128, by chance? Can you send me the IDBs? The "duplication" can in principle happen if there's another call to 0x63c52FA9, but the rendering of the graph is clearly "odd". I'm not sure yet, whether this is a bug in the graph display or an actual exporter bug.

togacure commented 8 years ago

Is this from CVE-2014-4128

No, it is bug-independent function, to example.

The "duplication" can in principle happen if there's another call to 0x63c52FA9, but the rendering of the graph is clearly "odd". I'm not sure yet, whether this is a bug in the graph display or an actual exporter bug.

Sorry, I tried, but was unable to allocate minimal sample, to trigger this. I do not understand what has caused this behavior, in terms of both function analysis framework in IDA deliver exactly the same result. In both cases BB functions are arranged with a separation from each other (seen in the address of the value). In both cases, error analysis highlights IDA stack frame, without good apparent reason. A minimum sample always ends with an unconditional jump is in a separate BB, and no duplicates.

Can you send me the IDBs?

To play a context I can say, on what files I get such a result, IDA6.9 analysis. If that is not enough - I share idb on file-share.

cblichmann commented 8 years ago

Yes, please share the IDB.

togacure commented 8 years ago

Sent sample to private. Test environment:

other are not tested. Stable replay on 2 iteration.

togacure commented 8 years ago

6 - This bug does not check for the 9th binexport build.

cblichmann commented 8 years ago

Just as a head-up, I won't be able to work on this for the next few weeks (travelling). Maybe someone else can fill in for me.

cblichmann commented 7 years ago

Finally had a look at this. IDA 6.95 gives this result for KB3134814.idb:

screenshot from 2017-04-19 15 46 46

This is just a side-effect of the stack-pointer analysis failing because of the function itself being used as a function chunk in dozens of other places in the binary.

BinExport exports this corresponding raw dump (check with binexport2dump tool):

636aff87 ; CImplAry::DeleteAll(void)
636aff87 ; ----------------------------------------
636aff87                 mov edi, edi
636aff89                 push esi
636aff8a                 mov esi, ecx
636aff8c                 test b1 ds:[esi], b1 0x2
636aff8f                 jnz 0x63c52fb8
636aff95 ; ----------------------------------------
636aff95                 jmp 0x63c52fa9
63c52fa9 ; ----------------------------------------
63c52fa9                 mov ecx, ds:[esi+0x8]
63c52fac                 call ?ProcessHeapFree@@YGHPAX@Z
63c52fb1                 and ds:[esi+0x8], b1 0x0
63c52fb5                 and ds:[esi], b1 0x3
63c52fb8 ; ----------------------------------------
63c52fb8                 and ds:[esi+0x4], b1 0x0
63c52fbc                 pop esi
63c52fbd                 retn

Which is clearly correct. What we could try is to force IDA to forcibly set the stack pointer difference before each jump into this function to zero and re-export. Since this occurs in dozens of places, I haven't done this.

I'll close as "Won't fix", feel free to reopen in BinNavi, in which case we'll investigate there.