upgrade rules using updated script

[mr-tz]

#837 and all the comments broke the GitHub web UI...

Repeating the work now here using a script and fixing things locally - for the most part.

[mr-tz]

See 409da0b for some first updates to rules with subscopes. Is that what you had in mind @williballenthin @yelhamer?

[mr-tz]

I think all rules have been upgraded now.

I did not use the process scope in any rule so far.

The linter needs to use the changes from the dynamic-feature-extraction branch to pass.

[williballenthin]

this duplication is obviously unfortunate, but i understand why

[mr-tz]

potentially there's just a lot of duplication for basic block -> call

could we support basic block / call syntax to simplify this?

[williballenthin]

i looked at the first hundred or so - changes look good. what level of review would you like from me? do you need every change reviewed? or are you confident in the changes and we can trust that?

[mr-tz]

If you've looked at a dozen or so changes from each commit (as they roughly group changes) I'm fairly confident in these updates and don't think we need a detailed review of each rule.

[williballenthin]

ah, that's a great way to group changes. i'll do this review tomorrow. thanks!

[williballenthin]

(the GH PR review interface is basically unusable for this set of commits 🤦🏼 )

I reviewed commit-by-commit, limiting myself to about 20 changed files per commit. I found no blockers.

Things I took away:

1. good: comment why a flavor is unsupported
2. good: comments indicating a branch is for static/dynamic flavor
3. good: nest static/dynamic branches under a single OR to group logic
4. maybe we can rely on the default logical statement for a subscope to be "AND", which simplifies some rules. consider:

- call:
  - and:
    - api: foo
    - number: 1

versus:

- call:
  - api: foo
  - number: 1

things to check:

a. VMCI socket: should this be scope=call not scope=thread?
b. protected exception handling: check call counting works as desired
c. check if file exists: use scope=call?

---

sidebar: here's how i did the review commit by commit, mostly for my personal future reference

git switch --detach b05c54734b56dfc77c542b9df723ad88f5799678
git reset $(git merge-base HEAD HEAD~)

this checks out the commit i want to inspect, and then creates a workspace as if only the changes in that commit are active an uncommited. so, then i can do git status to see all the modified fields and git diff to see the diffs.

notably, i can use tools like tig, gitui, and vs code to review the diffs and stage/unstage to mark things i agree with.

[williballenthin]

@mr-tz please see dbefb55

[mr-tz]

Is VMCI_SOCKETS_GET_AF_VALUE a valid argument to socket?

The other changes look good, thank you!

[williballenthin]

via here: https://github.com/mandiant/capa/blob/210a13d94ea40114e01b40927c77c1c74047780f/capa/rules/__init__.py#L544

an instruction subscope acts implicitly like an and: so we can save a line/indentation by not adding those. same with call. maybe we can do a cleanup of all the rules in another PR.

[williballenthin]

Is VMCI_SOCKETS_GET_AF_VALUE a valid argument to socket?

ah, you're right, this is only passed to DeviceIoControl/ioctl. updated the logic to look like:

      - and:
        - os: windows
        - api: DeviceIoControl
        - number: 0x81032068 = VMCI_SOCKETS_GET_AF_VALUE
        - optional:
          - api: socket
      - and:
        - os: linux
        - api: ioctl
        - number: 0x7B8 = VMCI_SOCKETS_GET_AF_VALUE
        - optional:
          - api: socket

scope: basic block/call

[williballenthin]

i think this PR is ready for merge (once dynamic analysis is merged)!

[mr-tz]

For call scope the optional socket would never match and the scopes should still be thread (and function) IMO.

[mr-tz]

via here: https://github.com/mandiant/capa/blob/210a13d94ea40114e01b40927c77c1c74047780f/capa/rules/__init__.py#L544

an instruction subscope acts implicitly like an and: so we can save a line/indentation by not adding those. same with call. maybe we can do a cleanup of all the rules in another PR.

I don't see that implemented for call, but think we should add and simplify the rules.

[mr-tz]

rebased to master and:

• changed scopes again for VMCI socket: 5e2dae1
• updated scopes for new rules: 5430889
• graduated rule: 305adfd