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

fix several bugs in array type handling in dotnet module #2064

Merged
merged 1 commit into from
May 2, 2024

Conversation

vthib
Copy link
Contributor

@vthib vthib commented Apr 7, 2024

Several bugs were present in the parsing and generation of a string to represent an array type, notably when the "lo_bound" value is set.

  • The "read_blob_signed" was buggy for values outside the [-2**6, 2**6-1], the sign edition did not use the right bitmask, and the type used was unsigned.

  • The display when lo_bound != 0 was buggy:

    • size=5, lobound=0 => should be 5, this was ok
    • size=5, lobound=1 => should be 1...5, this was buggy and displayed 1...6. The "range format" is inclusive.
  • 0 should still be displayed if size is 0. Only when size is unset should it be left out. So an array declared as [5,0,3] should be displayed the same way, and not as [5,,3].

@plusvic
Copy link
Member

plusvic commented Apr 8, 2024

@vthib do you have some .NET files that trigger these issues. I want to have some reference files and check that this is correctly done on YARA-X as well.

@vthib
Copy link
Contributor Author

vthib commented Apr 8, 2024

Yes, I crafted some files for testing in boreal, one of which test array sizes: types2.dll

lo bound values on arrays cannot be used in C# files afaik, but they can easily be tested with MSIL files. The aforementioned file was generated from this source code: types2.cil

This makes it easy to compare the syntax used in the MSIL file with the type strings generated in the yara module.

Several bugs were present in the parsing and generation of a string to
represent an array type, notably when the "lo_bound" value is set.

- The "read_blob_signed" was buggy for values outside the
  [-2**6, 2**6-1], the sign edition did not use the right bitmask,
  and the type used was unsigned.

- The display when lo_bound != 0 was buggy:

  - size=5, lobound=0 => should be `5`, this was ok
  - size=5, lobound=1 => should be `1...5`, this was buggy and displayed
    `1...6`. The "range format" is inclusive.

- 0 should still be displayed if size is 0. Only when size is unset
  should it be left out. So an array declared as `[5,0,3]` should be
  displayed the same way, and not as `[5,,3]`.
@vthib vthib force-pushed the fix-dotnet-array-type branch from 1685942 to 953df53 Compare April 30, 2024 20:49
@vthib
Copy link
Contributor Author

vthib commented Apr 30, 2024

I fixed an unnecessary change, should be ok now

@plusvic plusvic merged commit 2479a71 into VirusTotal:master May 2, 2024
10 checks passed
plusvic added a commit to VirusTotal/yara-x that referenced this pull request May 2, 2024
@vthib vthib deleted the fix-dotnet-array-type branch May 3, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants