Skip to content

fix: use Valuelength for parseString instead of whole String size#96

Merged
ayoubfaouzi merged 3 commits into
saferwall:mainfrom
Snshadow:fix/parse_string
Jul 1, 2024
Merged

fix: use Valuelength for parseString instead of whole String size#96
ayoubfaouzi merged 3 commits into
saferwall:mainfrom
Snshadow:fix/parse_string

Conversation

@Snshadow
Copy link
Copy Markdown
Contributor

@Snshadow Snshadow commented Jun 18, 2024

If a resource has a empty string, parsing value of string is parsed incorrectly(getting a next String part as string).
As String.ValueLength is length in utf-16 string without null value, use 2*(s.ValueLength+1) instead.

example

  • Empty value causes incorrectly parsed data.
    • Looks like the null value is skipped when getting end of string due to aligning offset with dword(4 bytes), and not being found by bytes.Index() in DecodeUTF16String().
"file_description": ">\u000f\u0001FileVersion",

If a resource has a empty string, parsing value of string is parsed incorrectly(getting a next String part as string).
As String.ValueLength is length in utf-16 string without null value, use 2*(s.ValueLength+1).
@Snshadow Snshadow changed the title fix: use Valuelength for parseString instead of whole string length fix: use Valuelength for parseString instead of whole String size Jun 18, 2024
@ayoubfaouzi
Copy link
Copy Markdown
Member

Hello @Snshadow

Sorry for my late reply but great catch !

Do you have a test sample or a hash to test this ? would be nice to add a test for this one or just provide me a hash and I can add it.

Cheers.

@Snshadow
Copy link
Copy Markdown
Contributor Author

Snshadow commented Jul 1, 2024

I added the sample file in test directory and test case in version_test.go, and the test passes with the fixed code.

@ayoubfaouzi
Copy link
Copy Markdown
Member

Beautiful ! LGMT, merging it now, I will push a new tag now.

@ayoubfaouzi ayoubfaouzi merged commit 371d03a into saferwall:main Jul 1, 2024
@Snshadow Snshadow deleted the fix/parse_string branch July 1, 2024 00:43
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