Skip to content

Fix Ghidra serializing negative 64-bit addresses#474

Merged
whyitfor merged 8 commits into
redballoonsecurity:masterfrom
rbs-afflitto:bugfix/ghidra_kernel_address_space
Jun 7, 2024
Merged

Fix Ghidra serializing negative 64-bit addresses#474
whyitfor merged 8 commits into
redballoonsecurity:masterfrom
rbs-afflitto:bugfix/ghidra_kernel_address_space

Conversation

@rbs-afflitto
Copy link
Copy Markdown
Collaborator

@rbs-afflitto rbs-afflitto commented Jun 4, 2024 •
edited
Loading

One sentence summary of this PR (This should go in the CHANGELOG!)

  • Fix Ghidra serializing negative 64-bit addresses

Link to Related Issue(s)
N/A

Please describe the changes in your request.
The OFRAK Ghidra scripts use %d string format of long integers when building JSON messages. In Java, this will be a signed decimal, but OFRAK expects an unsigned decimal. This causes problems in 64-bit Linux kernels, where commonly the address space is in the negative range. This change uses Long.toUnsignedString to format all long integers as unsigned before serializing to JSON. This PR also adds a test case which uses an elf with a negative address range.

Anyone you think should look at this, specifically?
@whyitfor @SamL98

@rbs-afflitto rbs-afflitto self-assigned this Jun 4, 2024
whyitfor
whyitfor requested changes Jun 5, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@whyitfor whyitfor left a comment

Choose a reason for hiding this comment

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

Please add a changelog to this MR

@rbs-afflitto rbs-afflitto requested a review from whyitfor June 5, 2024 16:22
@whyitfor whyitfor requested a review from SamL98 June 6, 2024 02:48
whyitfor
whyitfor requested changes Jun 6, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@whyitfor whyitfor left a comment

Choose a reason for hiding this comment

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

Looks good -- see a few minor comments/suggestions.

Comment thread disassemblers/ofrak_ghidra/CHANGELOG.md Outdated
Comment thread ofrak_core/pytest_ofrak/patterns/basic_block_unpacker.py Outdated
Comment thread ofrak_core/pytest_ofrak/patterns/code_region_unpacker.py
@SamL98
Copy link
Copy Markdown
Contributor

SamL98 commented Jun 6, 2024

LGTM. We should really use an actual JSON library in the ghidra scripts eventually.

whyitfor
whyitfor approved these changes Jun 7, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@whyitfor whyitfor left a comment

Choose a reason for hiding this comment

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

LGTM

@whyitfor whyitfor merged commit d5c20d7 into redballoonsecurity:master Jun 7, 2024
ANogin pushed a commit to ANogin/ofrak that referenced this pull request Aug 20, 2024
@rbs-afflitto
Fix Ghidra serializing negative 64-bit addresses (redballoonsecurity#474
724936f 
)

* Fix Ghidra serializing negative 64-bit addresses

* Lint

* Fix lint

* Fixed pre-commit hook

* Add bb data range for capstone test

* Update changelog

* Update changelog

* Add description of new test cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@whyitfor whyitfor whyitfor approved these changes

+1 more reviewer

@SamL98 SamL98 SamL98 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rbs-afflitto rbs-afflitto

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rbs-afflitto @SamL98 @whyitfor