capa icon indicating copy to clipboard operation
capa copied to clipboard

emit number(0) (offset(0)??) for instructions like "XOR EAX, EAX"

Open mike-hunhoff opened this issue 9 months ago • 22 comments

see https://github.com/mandiant/capa-rules/pull/993#issuecomment-2711090176

mike-hunhoff avatar Mar 10 '25 16:03 mike-hunhoff

good idea. i tend to think 'number: 0' we should definitely add. offset i'm not so sure.

williballenthin avatar Mar 10 '25 17:03 williballenthin

As capa doesn’t track register values, so number: 0 isn’t detected for indirect zeroing. I had tried adding directly bytes pattern - bytes: 33C050 in the mentioned pr, but it still wont work. We can try to match the semantic intent using mnemonics and operands but the output would be same most prolly. Any other leads on how i can proceed with this ?

dhruvak001 avatar Mar 12 '25 00:03 dhruvak001

Adding number(0) makes sense. As a downside this will create lots of additional features potentially affecting performance?!

Maybe we can create stats/benchmarks for a few samples to verify.

mr-tz avatar Mar 12 '25 09:03 mr-tz

As a downside this will create lots of additional features potentially affecting performance?!

I suspect we'd see this feature about once per function, since I've noticed many compilers will reserve a general purpose register for the zero value, when possible. Sometimes there might be a couple instances per function.

imho, this probably won't be a noticable amount of garbage, but I agree we should figure out a way to benchmark and verify.

williballenthin avatar Mar 18 '25 12:03 williballenthin

Hey, @williballenthin, @mike-hunhoff and @mr-tz . I would like to work on this issue. I have explored how a few capa backends extract features from the file. I am thinking of using cProfile and snakeviz for benchmarking and analyzing. Could you please give your suggestions/opinions for the same? Also, let me know if you recommend using some other tool for profiling/benchmarking.

v1bh475u avatar Mar 18 '25 12:03 v1bh475u

I would start by using something like hyperfine to benchmark the invocation of capa against a fairly complex sample, like mimikatz. Only if there's a measurable difference before/after the changes should we dive into the line-level profiling. I think its likely that the performance delta is so small it is hidden by random noise.

williballenthin avatar Mar 18 '25 15:03 williballenthin

Image

Image

The first one in each image is from current master branch and second one is from my implementation for xor <reg>, <reg> in vivisect backend. My implementation:

    if insn.mnem == "xor" and insn.opers[0].isReg() and insn.opers[1].isReg() and insn.opers[0].reg == insn.opers[1].reg:
        # for pattern like:
        #
        #     xor eax, eax
        #
        yield Number(0), ih.address
    # this is for both x32 and x64
    if not isinstance(oper, (envi.archs.i386.disasm.i386ImmOper, envi.archs.i386.disasm.i386ImmMemOper)):
        return

v1bh475u avatar Mar 18 '25 19:03 v1bh475u

great @v1bh475u! the implementation looks nice and straightforward - good job.

i'm not sure how to read the screenshots, since there's two of them :-) the second one looks like there's no real change in performance, but the first one might show something? what's the difference between the two screenshots?

williballenthin avatar Mar 18 '25 19:03 williballenthin

I think in the first screenshot, the 2nd test might have been influenced by other processes running on my machine. Let me retry to ensure that.

v1bh475u avatar Mar 18 '25 20:03 v1bh475u

one thing that might also come into play is CPU throttling, which may penalize whatever test is going second (when CPU is warmer so might be throttled). so, i'd recommend doing the testing on dedicated non-laptop hardware (if possible), or alternating the tests more so that both cases have a chance to run "second", or letting the system cool down before running new tests.

williballenthin avatar Mar 18 '25 20:03 williballenthin

I think in the first screenshot, the 2nd test might have been influenced by other processes running on my machine. Let me retry to ensure that.

Image

The average time for each test has increased but the difference is still barely 1-2 seconds.

v1bh475u avatar Mar 18 '25 21:03 v1bh475u

one thing that might also come into play is CPU throttling, which may penalize whatever test is going second (when CPU is warmer so might be throttled). so, i'd recommend doing the testing on dedicated non-laptop hardware (if possible), or alternating the tests more so that both cases have a chance to run "second", or letting the system cool down before running new tests.

Agreed. I will run further tests and update them here soon.

v1bh475u avatar Mar 18 '25 21:03 v1bh475u

Image The above is the result on running the test multiple times on my server machine with following specs:

  • OS: Ubuntu 22.04.5 LTS x86_64
  • Kernel: Linux 6.8.0-52-generic
  • CPU: Intel(R) Core(TM) i7-7700 (8) @ 4.20 GHz
  • Memory: 46.91 GiB The 1st test was a bit skewed due to some heavy process going on the server. After that, all tests are stable. On average, the branch with new implementation took $63.184 \pm 0.352$ s while the current master branch took $62.821 \pm 0.285$ s. Hence, in conclusion, I feel the addition of this feature increases the overhead barely by 1-2 seconds in the worst case.

v1bh475u avatar Mar 19 '25 00:03 v1bh475u

@williballenthin @mike-hunhoff @mr-tz , shall I start working on modifications similar to above one for the backends which I can test for?

v1bh475u avatar Mar 20 '25 12:03 v1bh475u

yes, please do!

and, please add a test case to demonstrate the new functionality.

williballenthin avatar Mar 20 '25 13:03 williballenthin

@williballenthin , I have an older version of binaryninja given by one of my friends (version: 3.5.4526 Personal) and it is giving ImportError: cannot import name 'ILException' from 'binaryninja'. Is the current backend not meant to support this version binaryninja APIs?

v1bh475u avatar Mar 20 '25 18:03 v1bh475u

Most likely, @xusheng6 did various updates around feature extraction.

mr-tz avatar Mar 20 '25 18:03 mr-tz

Also, we want the tests to run as fast as possible, right?

v1bh475u avatar Mar 20 '25 19:03 v1bh475u

@williballenthin , I have an older version of binaryninja given by one of my friends (version: 3.5.4526 Personal) and it is giving ImportError: cannot import name 'ILException' from 'binaryninja'. Is the current backend not meant to support this version binaryninja APIs?

Yeah version 3.5 is a bit old. Could you please test it against version 4.2 and see if it works?

xusheng6 avatar Mar 21 '25 07:03 xusheng6

@v1bh475u if you don't have a license for Binary Ninja, don't worry about it. Once you have a unit test and an implementation for IDA, I'd be happy to handle the Binary Ninja side. I expect it's only a few lines, like for IDA.

williballenthin avatar Mar 21 '25 07:03 williballenthin

@williballenthin I have made the PR and also added test case for xor eax, eax instruction. For the binaryninja backend, due to instruction matching occuring in LLIL, xor eax, eax transforms into eax = 0 which is handled by the existing logic. I am currently working on IDA backend.

v1bh475u avatar Mar 21 '25 10:03 v1bh475u

+1 for this feature, just ran into this while working on https://github.com/mandiant/capa-rules/pull/1046. i want to match on 3 arguments to a function being 0 but MSVC is emitting xor's to zero out the registers:

xor     r9d, r9d
xor     r8d, r8d
xor     edx, edx
mov     rcx, [rbp+230h+arg_0]
call    cs:NtFsControlFile

zdwg42 avatar May 12 '25 17:05 zdwg42