mandiant / capa-rules

Standard collection of rules for capa: the tool for enumerating the capabilities of programs
https://github.com/mandiant/capa/
Apache License 2.0
517 stars 157 forks source link

PLUGX: make more restrictive to fix FP #843

Closed williballenthin closed 10 months ago

williballenthin commented 10 months ago

closes #842

williballenthin commented 10 months ago

@Still34 FYI

williballenthin commented 10 months ago

looks like i need to review the linter failure before this is merged. please review the intent of the logic change in the meantime.

Still34 commented 10 months ago

Shoot, I'll take a look in a bit

Still34 commented 10 months ago

Looks perfectly fine to me as well

williballenthin commented 10 months ago

when I proposed the changes here I imagined (without looking, sorry) that there'd be a large command ID dispatcher that had a bunch of the command IDs referenced in the same function, using a case/switch statement. turns out this is not the case for plugx. instead, we have individual functions that handle each command, parsing or creating the protocol, like:

image

note that we see:

mov [base+0], TIMESTAMP
mov [base+4], COMMAND

also: along the way i did find a FP in the same sample: see how 0x4000 is used as a bitmask in a related function, not as the command ID:

image

so, i'll propose updating the rule to:

- and:
  - instruction:
    - mnemonic: mov
    - operand[0].offset: 0
    - or:
      - operand[1].number: TIMESTAMP1
      - operand[1].number: TIMESTAMP2
  - instruction:
    - mnemonic: mov
    - operand[0].offset: 4
    - or:
      - operand[1].number: COMMAND1
      - operand[1].number: COMMAND2

i think its appropriate to set the scope to basic block, too.

williballenthin commented 10 months ago

image

williballenthin commented 10 months ago

@Still34 @mr-tz would you take another peek, please?

Still34 commented 10 months ago

Throw this timestamp in there as well 0x20140613, also I could not get the match to pass with sample edfff708808ef3a5453dc7713d306b20 @ 0x1000A258 (and yes the timestamp is added to the local rule).

mr-tz commented 10 months ago

with the added value it matches for me: image

williballenthin commented 10 months ago

@Still34

also I could not get the match to pass

would you double check using -vv?

it looks like today we're filtering out malware family rules from the default display. im not quite sure why we're doing that. do you remember @mr-tz?

mr-tz commented 10 months ago

I'm not sure why. Maybe we wanted to render it separately...