dncil icon indicating copy to clipboard operation
dncil copied to clipboard

add support for basic blocks

Open mike-hunhoff opened this issue 1 year ago • 8 comments

mike-hunhoff avatar Feb 28 '23 22:02 mike-hunhoff

See #55 for progress.

mike-hunhoff avatar Mar 22 '24 15:03 mike-hunhoff

Hello @mike-hunhoff , Do let me know if and how I can be of help with this issue?

akhilguruprasad22 avatar Apr 10 '24 16:04 akhilguruprasad22

@akhilguruprasad22 I'd love your help here! I've got a draft PR open that you can continue to develop. It's almost at the finish line but there is some unaddressed feedback that needs to be investigated. I'd recommend you start by reviewing the draft PR + unaddressed feedback. Please let me know if you have any questions!

mike-hunhoff avatar Apr 10 '24 17:04 mike-hunhoff

I've assigned this issue to you for now. No pressure, if you decide not to move forward please let me know and I'll remove you 😄

mike-hunhoff avatar Apr 10 '24 17:04 mike-hunhoff

Got it @mike-hunhoff , thank you. I shall look into this right away.

akhilguruprasad22 avatar Apr 11 '24 16:04 akhilguruprasad22

Hello @mike-hunhoff , Apologies for the unavoidable delay in resolving this. I won't be able to raise a PR with the changes (95% completed) until the next weekend. I really hope this won't be an issue.

I do have a few questions for you:

  1. Should blocks which have no predecessors i.e. instructions following unconditional branches be considered block leaders? Every blog, including the url provided as a comment, I've come across state that they should be. @williballenthin mentions this on the capa PR review. In our tests, the exception handler block falls into this category. Hence using this as an example, we can argue that they are valid block leaders. Wanted to get this cleared up since a suggested comment makes a mention of this.
  2. There is a suggestion to include your logic regarding connecting blocks as comments: https://github.com/mandiant/capa/pull/1326#discussion_r1120866293 I was wondering whether it'd be acceptable to rephrase your comment or if it'd be prefered to have them in the code verbatim.

There were also some assumptions needing verification:

  1. does .NET support non-returning functions:

The 6th point in the following snippet from the ECMA CLI spec file answers this:

Control transfer CLI

  1. .NET doesn't support tail calls, i.e. jmp to other routine:

While the CLR does support tail calls, the C# compiler doesn't. https://blog.objektkultur.de/about-tail-recursion-in-.net/

  1. .NET doesn't support shared function chunks, i.e. two different entry points to the same function body:

I'm yet to document sources which prove this.

akhilguruprasad22 avatar Apr 20 '24 00:04 akhilguruprasad22

@akhilguruprasad22 thank you for all of your research here. To answer your questions

Should blocks which have no predecessors i.e. instructions following unconditional branches be considered block leaders? Every blog, including the url provided as a comment, I've come across state that they should be. @williballenthin mentions this on the capa PR review. In our tests, the exception handler block falls into this category. Hence using this as an example, we can argue that they are valid block leaders. Wanted to get this cleared up since a suggested comment makes a mention of this.

Yes, instructions following unconditional branches should be considered block leaders with the most likely case being exception handlers. dncil stores exception handler information in https://github.com/mandiant/dncil/blob/main/dncil/cil/body/init.py#L40 that includes relevant instruction offsets. We could use this information to detect additional block leaders or default to adding instructions that follow instructions w/out fallthrough as block leaders, e.g. from ECMA I.12.4.2.8 .1

[Note: Most instructions can allow control to fall through after their execution—only unconditional branches, ret, jmp, leave(.s), endfinally, endfault, endfilter, throw, and rethrow do not. Call instructions do allow control to fall through, since the next instruction to be executed in the current method is the one lexically following the call instruction, which executes after the call returns. end note]

There is a suggestion to include your logic regarding connecting blocks as comments: https://github.com/mandiant/capa/pull/1326#discussion_r1120866293 I was wondering whether it'd be acceptable to rephrase your comment or if it'd be prefered to have them in the code verbatim.

Please rephrase as needed.

mike-hunhoff avatar Apr 25 '24 22:04 mike-hunhoff

Hello @mike-hunhoff , Thank you for resolving my queries. I have raised a PR here, awaiting further review.

akhilguruprasad22 avatar Apr 29 '24 09:04 akhilguruprasad22