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 crash while editing asm. #3400

Merged
merged 1 commit into from
Jan 10, 2025
Merged

Fix crash while editing asm. #3400

merged 1 commit into from
Jan 10, 2025

Conversation

karliss
Copy link
Member

@karliss karliss commented Jan 10, 2025

Empty input field resulted in empty byteArray and constData pointing to null, which rz_hex_bin2str didn't expect.

Your checklist for this pull request

Detailed description

Thanks @ravenexp for testing and debugging the problem.

Not only the function didn't handle empty QByteArray, there was a second hidden problem. The function reserved memory for 2n bytes but rz_hex_bin2str writes a null terminated string thus requiring 2n+1 bytes of space. The extra null byte write was probably also what caused the crash, just for non empty arrays single byte buffer overflow would most of the time not cause easily observable sideeffects.

Test plan (required)

  1. open executable
  2. enable cache mode
  3. in disasm widget edit instruction
    • change constant -> observe that there are no errors and byte preview updates
    • delete all text -> should not crash
    • write a different valid instruction for example nop -> bytes should be updated

Closing issues

Closes #3374, closes #3356

Empty input field resulted in empty byteArray and constData pointing to null, which rz_hex_bin2str didn't expect.
@karliss karliss added BUG crash Anything that causes Cutter to crash labels Jan 10, 2025
Copy link
Member

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rz_hex_str2bin function needs some work tbh. But it does so much more then it says (you can have comments in the string!), that it is scary to touch it

@karliss
Copy link
Member Author

karliss commented Jan 10, 2025

The rz_hex_str2bin function needs some work tbh. But it does so much more then it says (you can have comments in the string!), that it is scary to touch it

For a moment you got me scared, but in this case it was bin2str not not str2bin.

@karliss karliss merged commit db140ee into rizinorg:dev Jan 10, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG crash Anything that causes Cutter to crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing assembly instruction dialog causes segmentation fault Crash when cache mode
2 participants