Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for basic blocks #52

Open
mike-hunhoff opened this issue Feb 28, 2023 · 8 comments
Open

add support for basic blocks #52

mike-hunhoff opened this issue Feb 28, 2023 · 8 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mike-hunhoff
Copy link
Collaborator

No description provided.

@mike-hunhoff mike-hunhoff self-assigned this Feb 28, 2023
@mike-hunhoff mike-hunhoff added the enhancement New feature or request label Jul 10, 2023
@mike-hunhoff mike-hunhoff added the help wanted Extra attention is needed label Mar 22, 2024
@mike-hunhoff mike-hunhoff removed their assignment Mar 22, 2024
@mike-hunhoff
Copy link
Collaborator Author

See #55 for progress.

@akhilguruprasad22
Copy link
Contributor

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

@mike-hunhoff
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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 😄

@akhilguruprasad22
Copy link
Contributor

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

@akhilguruprasad22
Copy link
Contributor

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:
    dotnet: add support for basic blocks capa#1326 (comment)
    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.

@mike-hunhoff
Copy link
Collaborator Author

@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:
mandiant/capa#1326 (comment)
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.

@akhilguruprasad22
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants