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 get instr normalization #9

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

Ninja3047
Copy link
Contributor

@Ninja3047 Ninja3047 commented Feb 13, 2024

Attempting to patch an PIE ARM thumb binary (attached) produces the following error.

Traceback (most recent call last):
  File "/workdir/tests/test_arm.py", line 75, in test_insert_instruction_pie_patch
    self.run_one(
  File "/workdir/tests/test_arm.py", line 192, in run_one
    p.apply_patches()
  File "/workdir/src/patcherex2/patcherex.py", line 57, in apply_patches
    patch.apply(self)
  File "/workdir/src/patcherex2/patches/instruction_patches.py", line 51, in apply
    p.utils.insert_trampoline_code(
  File "/workdir/src/patcherex2/components/utils/utils.py", line 18, in insert_trampoline_code
    assert force_insert or self.is_valid_insert_point(
  File "/workdir/src/patcherex2/components/utils/utils.py", line 103, in is_valid_insert_point
    return self.get_instrs_to_be_moved(addr) is not None
  File "/workdir/src/patcherex2/components/utils/utils.py", line 95, in get_instrs_to_be_moved
    if not self.is_movable_instruction(insn_addr):
  File "/workdir/src/patcherex2/components/utils/utils.py", line 108, in is_movable_instruction
    asm = self.p.disassembler.disassemble(insn, addr, is_thumb=is_thumb)[0]
IndexError: list index out of range

printf_pie.zip

This PR fixes this issue by making sure to check for the thumbness first before denormalizing the address and adds a small unit test that tests for this issue.

@DennyDai
Copy link
Collaborator

DennyDai commented Feb 13, 2024

@Ninja3047 Thank you for the fix! It looks good to me. Please ignore the private-test that's actually a CI issue. Could you provide the source code and compiler flags so that we may reuse it for other architectures in the future? Just pasting them here as a reply would be great.

@DennyDai DennyDai merged commit 15824c2 into purseclab:main Feb 13, 2024
3 of 4 checks passed
@Ninja3047
Copy link
Contributor Author

@Ninja3047 Thank you for the fix! It looks good to me. Please ignore the private-test that's actually a CI issue. Could you provide the source code and compiler flags so that we may reuse it for other architectures in the future? Just pasting them here as a reply would be great.

FROM ubuntu:22.04

RUN apt-get update && apt-get install -y gcc-12-arm-linux-gnueabihf
#include <stdio.h>

int main(int argc, char* argv[]) {
    printf("%s", "Hi");
    return 0;
}

compile flags are the default ubuntu compiler flags

arm-linux-gnueabihf-gcc-12 -o printf_pie printf_pie.c

@Ninja3047 Ninja3047 deleted the fix-get-instr-normalization branch February 13, 2024 22:31
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