incorrect execution of GetLastError, wrong set of assembly instructions executed #44
Comments
|
I've implemented a full hook for However, before submitting a PR for this, we should try to assess what the deeper root cause of this issue is, as the incorrect set of assembly instructions are indeed execution when invoking |
mewmew
changed the title
Actually, I have a sample that depends on a full hook; where the sample sets an error and then later expects it to be set. Otherwise, it exists early without any of the interesting code being executed. Since However, we do need to figure out the root cause of this issue, as it's unnerving with inaccurate emulation (or disassembly). I can hook up another disassembler to verify if this is a Capstone issue. (I don't think it is, but definitely worth to verify.) |
|
I don't think it is capstone because return is incorrect, it was just the first thing that came to mind. Although, the gapstone lib needs to be upgraded to support capstone4 which I just submitted a PR for. I can't test it against my local data set until Monday. |
|
Ok, I just verified, this is not an issue with Capstone. Debug output of Debug output of The following patch was applied to rev d33dd0f to use the native Go disassembler for x86 instead of Capstone. diff --git a/windows/hooks.go b/windows/hooks.go
index 3881552..862a0ac 100644
--- a/windows/hooks.go
+++ b/windows/hooks.go
@@ -11,6 +11,7 @@ import (
"github.com/carbonblack/binee/util"
uc "github.com/unicorn-engine/unicorn/bindings/go/unicorn"
+ "golang.org/x/arch/x86/x86asm"
)
func (emu *WinEmulator) LoadHooks() {
@@ -267,8 +268,9 @@ func (self *Instruction) Address() string {
func (self *Instruction) Disassemble() string {
buf, _ := self.emu.Uc.MemRead(self.Addr, uint64(self.Size))
- if inst, err := self.emu.Cs.Disasm(buf, 0, uint64(self.Size)); err == nil {
- return fmt.Sprintf("%s %s", inst[0].Mnemonic, inst[0].OpStr)
+ mode := int(8*self.emu.PtrSize)
+ if inst, err := x86asm.Decode(buf, mode); err == nil {
+ return inst.String()
}
return ""
} |
It may be worth considering to use the native Go disassembler for x86, instead of Gapstone, or at least to evaluate pros/cons. This is the disassembler used by the Go compiler and toolchain. https://godoc.org/golang.org/x/arch/x86/x86asm For a patch to use |
|
@kgwinnup, one quick question. How does [1] 0x00401018: jmp dword ptr [0x402000]
[1] 0x2025e0bf: P kernel32.dll:GetLastError() = 0xb0010000
[1] 0x2025e0c0: push esp
[1] 0x2025e0c1: inc esp |
I had no idea this even existed. I agree, this is definitely worth using over gapstone. I would like to keep dependencies to a minimum. |
I am curious of your thoughts on this. This all starts at the location below, and i'll try and give a short explanation of the process. https://github.com/carbonblack/binee/blob/master/windows/loader.go#L787 First the PE file's imports are looped over and each DLL is loaded recursively and stored into a map in WinEmulator. There are set of maps that have mappings from Name->Hook, Addr->Hook, and Dll->RealDllName https://github.com/carbonblack/binee/blob/master/windows/winemulator.go#L82 At this time all the imports names are resolved into a map (see link above) which is later used for Hook lookups. Before each DLL is loaded into unicorn memory, the image address needs to be updated. Before this starts, there is a "counter" called NextLibAddress (located as a property of WinEmulator) that always starts at a static location based on whether the PE is 32 or 64 bit. This is in the WinEmulator.Load (previously New) function. After each DLL is loaded from host disk, and before mapped into memory, the image address needs to be updated (this also updates other properties of the PE file). This is always the NextLibAddress and after mapping into unicorn, the NextLibAddress is incremented to point to the next available location for a DLL to be mapped in. https://github.com/carbonblack/binee/blob/master/windows/loader.go#L227 |
I don't know if this is going to be possible without implementing something specific to mapping a function->original address. My workflow for this, while not the best, is to lookup the function manually in whatever RE tool (I use radare2) and then jumping to the real implementation. I am wondering if it would be useful to have a map as your looking for, and displaying that address instead of the temp emulator address. The temp address 0x2XXXXXXXX is not particular useful in anyway right now. Maybe later if debugger support is added. Possibly a good place to capture the original function and DLL base address is: https://github.com/carbonblack/binee/blob/master/windows/loader.go#L227 |
I see. Well, it should be relatively easy for users to simply binary search for the address to determine the original library, and then check the delta against the base address and thus compute the "original" address using this information.
That's my workflow as well. And it works great, except for the case of
Displaying the original address would indeed be very useful! Then you can just check what DLL the code is running in and jump directly to the address of any given instruction in say IDA/radare2. |
|
Oh, and thanks for the write-up. I appreciate it. |
I was running
bineelast night, and on one of my samples the invocation ofGetLastErrorwould result in incorrect execution (eventually resulting in an invalid read).I used IDA to verify the implementation of
GetLastError, and for some reason,bineeis executing another set of assembly instructions, rather than the actual assembly instructions of theGetLastErrorfunction (as present inkernel32.dll).To help troubleshoot, I've created a minimal test case (
b.exe). Find source code, build instructions and binary attached below.Results of running
bineeonb.exe:Contents of
win.yaml:$ cat win.yaml root: "/home/u/_share_/xp/b.exeattachment: b.tar.gzReference disassembly of
GetLastErrorfrom IDA:Edit: note that the return value of
GetLastErrorreported bybineeis incorrect ([1] 0x2025e0bf: P kernel32.dll:GetLastError() = 0xb0010000), it should be0x2anot0xb0010000). Furthermore, not that the first assembly instruction ofGetLastErroris reported bybineeas[1] 0x2025e0bf: dec esi, but should be.text:7C830759 mov eax, large fs:18h.The text was updated successfully, but these errors were encountered: