NationalSecurityAgency / ghidra

Ghidra is a software reverse engineering (SRE) framework
https://www.nsa.gov/ghidra
Apache License 2.0
51.42k stars 5.86k forks source link

6809 (6x09.sinc) : Inaccurate JSR and JMP implementations #4630

Closed sad-dev closed 1 year ago

sad-dev commented 2 years ago

Describe the bug

image image

is incorrect, according to https://www.maddes.net/m6809pm/appendix_a.htm

image

This results in inaccurate decompilation/disassembly e.g.

image

image

GhidorahRex commented 2 years ago

This looks like a duplicate of https://github.com/NationalSecurityAgency/ghidra/issues/4600 at first, but that one has a disassembly error vs this one disassembling incorrectly.

GhidorahRex commented 2 years ago

This doesn't seem wrong, unless I'm misunderstanding your issue. 6809_jmp_screenshot

I have 7e f0 00 which, according to the instruction manual should branch to address 0xf000. The instruction does branch to f000 in my test binary. What address is your JMP instruction branching to?

sad-dev commented 2 years ago

Mine reads a pointer from f000 and tries to branch there.

I was following this writeup at https://mahaloz.re/2022/09/23/0ctf22-rev.html

a.) Get game.bin from https://github.com/shellphish/writeups/blob/main/challenges/rev/m68k/0CTF22-Vintage/game.bin b.) Load as Raw Binary, 6809, say no to autoanalysis and add an RWX memory block at 0x8000 of size 0x8000, initialize with 0x33 c.) Creating functions at 001e, 0974, 0A5F, etc. causes errors like these image

d.) Observe that JSR has the same problem at 004d - creating a reference to 3460. instead of branching to 2f3c Marking the initial memory block as RX will cause 3460 to appear in the decompiler window as well.

sad-dev commented 2 years ago

A second thing that I'm not as sure about is the implementation of push and pull in SLEIGH:

image

It doesn't appear that pull1 and push1 are symmetric? Contrast with ia.sinc (x86)

image

The pull2 and push2 also look a bit sus - but I hardly touch sleigh so would not be surprised if its fine

GhidorahRex commented 2 years ago

The pull1 macro is definitely off. It should be decremented then stored. I'll have to investigate the pull2 macro because it definitely feels wrong, but it might be an endianness issue.

I see the JMP issue now, as well (line 379 reads the data at the address of the immediate instead of just exporting the immediate). I think it is a straightforward fix, but I want to make sure it won't break any other instructions.

If you want to test that fix out, replace 379 with: tmp:2 = imm16; export tmp;

In the meantime I'll analyze the rest of the sleigh file and make sure that doesn't cause other issues.

GhidorahRex commented 2 years ago

After having an opportunity to read through some of the documentation on the 6809, I found a programming reference that mentioned this issue:

Unlike most other instructions which use the Direct, Indexed and Extended addressing modes, the operand value used by the JMP instruction is the Effective Address itself, rather than the memory contents stored at that address (unless Indirect Indexing is used).

The same comment is included in the JSR instruction. Clearly, we're interpreting the indexing modes incorrectly for these instructions. The fix above will not work, since it messes with all the other extended address instructions.