search

NationalSecurityAgency / ghidra

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

68HC05: Incorrect decompilation and analysis of JMP loc,X jump tables #7064

Open philpem opened 1 month ago

philpem commented 1 month ago

Describe the bug Ghidra incorrectly interprets the JMP loc,X instruction as an indirected jump, when it is an indexed jump.

To Reproduce

For convenience, I've attached a Ghidra project archive: egret_2024_10_16.gar.zip

Disassemble the following block of code:

                             LAB_12f9                                        XREF[1]:     FUN_12ab:12c9(j)  
            12f9 b6 ba           LDA        DAT_00ba                                         = ??
            12fb a1 20           CMP        #0x20
            12fd 24 09           BCC        switchD_1305::caseD_20
            12ff b7 93           STA        DAT_0093                                         = ??
            1301 48              ASLA
            1302 bb 93           ADD        DAT_0093                                         = ??
            1304 97              TAX
                             switchD_1305::switchD
            1305 dc 13 16        JMP        switchD_1305::switchdataD_1316,X
                             switchD_1305::caseD_20                          XREF[4]:     12fd(j), 1305(j), 130d(j), 
                                                                                          1382(j)  
            1308 3f b5           CLR        DAT_00b5                                         = ??
            130a cd 14 aa        JSR        FUN_14aa                                         undefined FUN_14aa()
            130d 24 f9           BCC        switchD_1305::caseD_20
            130f a6 02           LDA        #0x2
            1311 cc 12 de        JMP        FUN_12de                                         undefined FUN_12de()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             LAB_1314                                        XREF[3]:     FUN_12ab:12ab(j), 
                                                                                          FUN_12ab:12b3(j), 
                                                                                          FUN_12ab:12b8(j)  
            1314 99              SEC
            1315 81              RTS
                             switchD_1305::switchdataD_1316                  XREF[1]:     1305(*)  
            1316 cc 16 58        JMP        FUN_1658                                         undefined FUN_1658()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1319
            1319 cc 16 65        JMP        FUN_1665                                         undefined FUN_1665()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_131c
            131c cc 16 82        JMP        FUN_1682                                         undefined FUN_1682()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_131f
            131f cc 16 b3        JMP        FUN_16b3                                         undefined FUN_16b3()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1322
            1322 cc 16 eb        JMP        FUN_16eb                                         undefined FUN_16eb()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1325
            1325 cc 17 07        JMP        FUN_1707                                         undefined FUN_1707()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1328
            1328 cc 17 24        JMP        FUN_1724                                         undefined FUN_1724()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_132b
            132b cc 17 41        JMP        FUN_1741                                         undefined FUN_1741()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_132e
            132e cc 17 b0        JMP        FUN_17b0                                         undefined FUN_17b0()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1331
            1331 cc 17 d8        JMP        FUN_17d8                                         undefined FUN_17d8()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1334
            1334 cc 17 f0        JMP        FUN_17f0                                         undefined FUN_17f0()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1337
            1337 cc 18 0b        JMP        FUN_180b                                         undefined FUN_180b()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_133a
            133a cc 18 2b        JMP        FUN_182b                                         undefined FUN_182b()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_133d
            133d cc 18 65        JMP        FUN_1865                                         undefined FUN_1865()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1340
            1340 cc 18 7a        JMP        FUN_187a                                         undefined FUN_187a()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1343
            1343 cc 18 b7        JMP        FUN_18b7                                         undefined FUN_18b7()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1346
            1346 cc 19 83        JMP        FUN_1983                                         undefined FUN_1983()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1349
            1349 cc 19 a6        JMP        FUN_19a6                                         undefined FUN_19a6()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_134c
            134c cc 19 b4        JMP        FUN_19b4                                         undefined FUN_19b4()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_134f
            134f cc 19 d1        JMP        FUN_19d1                                         undefined FUN_19d1()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1352
            1352 cc 19 ee        JMP        FUN_19ee                                         undefined FUN_19ee()
                             -- Flow Override: CALL_RETURN (CALL_TERMINATOR)
                             switchD_1305::switchdataD_1355
            1355 cc 19 ff        JMP        LAB_19ff
                             switchD_1305::switchdataD_1358
            1358 cc 1a 1c        JMP        LAB_1a1c
                             switchD_1305::switchdataD_135b
            135b cc 1a 32        JMP        LAB_1a32
                             switchD_1305::switchdataD_135e
            135e cc 1a 43        JMP        LAB_1a43
                             switchD_1305::switchdataD_1361
            1361 cc 1a 59        JMP        LAB_1a59
                             switchD_1305::switchdataD_1364
            1364 cc 1a 6e        JMP        LAB_1a6e
                             switchD_1305::switchdataD_1367
            1367 cc 1a 8b        JMP        LAB_1a8b
                             switchD_1305::switchdataD_136a
            136a cc 1a ed        JMP        LAB_1aed
                             switchD_1305::switchdataD_136d
            136d cc 1b 02        JMP        LAB_1b02
                             switchD_1305::switchdataD_1370
            1370 cc 1b 17        JMP        LAB_1b17
                             switchD_1305::switchdataD_1373
            1373 cc 1b 2b        JMP        LAB_1b2b

Initially Ghidra will disassemble this and mark the 0xCC bytes as data.

Select the JMP instruction and look at the matching code in the decompiler window. It will look like:

      if (DAT_00b9 == '\x01') {
        bVar3 = DAT_00ba < 0x20;
        DAT_0093 = DAT_00ba;
        switch(DAT_00ba) {
        case 0:
        case 1:
        case 2:
        case 3:
        case 4:
        case 5:
        case 6:
        case 7:
        case 8:
        case 9:
        case 10:
        case 0xb:
        case 0xc:
        case 0xd:
        case 0xe:
        case 0xf:
        case 0x10:
        case 0x11:
        case 0x12:
        case 0x13:
        case 0x14:
        case 0x15:
        case 0x16:
        case 0x17:
        case 0x18:
        case 0x19:
        case 0x1a:
        case 0x1b:
        case 0x1c:
        case 0x1d:
        case 0x1e:
        case 0x1f:
                    /* WARNING: Bad instruction - Truncating control flow here */
          halt_baddata();
        }

As Ghidra marks the first byte of the jump target as "case data" and marks the address 0xCC as "case code", it seems like Ghidra is misinterpreting the JMP nnnn,X instruction (opcode 0xDC) as if it were a hybrid of the JMP (nnnn,X) and JMP (nn,X) instructions -- in the sense that it reads the byte at the target, then jumps to it (an indirected jump), rather than jumping directly to the target.

Expected behavior Ghidra interprets the code above correctly, and follows the jump instruction to one of the jump table entries, correctly identifying the code which is executed.

Environment (please complete the following information):

philpem commented 1 month ago

This is a bug in https://github.com/NationalSecurityAgency/ghidra/blob/de7c3eaee2a4bc993a402e371b039c2bb2d6c545/Ghidra/Processors/MC6800/data/languages/6805.slaspec#L374 :

:JMP ADDRI  is (op=0xDC | op=0xEC | op=0xFC) ... & ADDRI
{
    goto [ADDRI];
}

The square brackets need to go:

:JMP ADDRI  is (op=0xDC | op=0xEC | op=0xFC) ... & ADDRI
{
    goto ADDRI;
}

There seems to be a similar bug in the indexed JSR instruction: https://github.com/NationalSecurityAgency/ghidra/blob/de7c3eaee2a4bc993a402e371b039c2bb2d6c545/Ghidra/Processors/MC6800/data/languages/6805.slaspec#L386

:JSR ADDRI  is (op=0xDD | op=0xED | op=0xFD) ... & ADDRI
{
    *:2 (SP-1) = inst_next;
    SP=SP-2; 
    call [ADDRI];
}
philpem commented 1 month ago

I've synced to master and tried the PR out on my local build. While it seems to fix the references, it causes other issues:

  1. Ghidra no longer detects and marks the jump table.
  2. The Stack analyser throws an assert on the 1305 dc 13 16 JMP LAB_1316,X instruction.
  3. The jump target (address 0x1316) is marked as a DATA reference in the References Editor - I think it should be COMPUTED_JUMP (and for the JSR version it should be COMPUTED_CALL).
  4. The decompiler complains when the JMP instruction is selected: Low-level Error: Could not find op at target address: (unique,0x00000500).

I don't think I'm doing anything wrong with the SLEIGH description, so this is puzzling. (though I'm used to the jump table detector missing jump tables)

The backtrace from the Stack analyser is:

Not a valid Address on instruction at 1305
ghidra.util.exception.AssertException: Not a valid Address on instruction at 1305
    at ghidra.program.util.SymbolicPropogator.applyPcode(SymbolicPropogator.java:1047)
    at ghidra.program.util.SymbolicPropogator.flowConstants(SymbolicPropogator.java:529)
    at ghidra.program.util.SymbolicPropogator.flowConstants(SymbolicPropogator.java:425)
    at ghidra.program.util.SymbolicPropogator.flowConstants(SymbolicPropogator.java:179)
    at ghidra.app.cmd.function.NewFunctionStackAnalysisCmd.createStackPointerVariables(NewFunctionStackAnalysisCmd.java:334)
    at ghidra.app.cmd.function.NewFunctionStackAnalysisCmd.analyzeFunction(NewFunctionStackAnalysisCmd.java:185)
    at ghidra.app.cmd.function.NewFunctionStackAnalysisCmd.applyTo(NewFunctionStackAnalysisCmd.java:113)
    at ghidra.app.cmd.function.NewFunctionStackAnalysisCmd.applyTo(NewFunctionStackAnalysisCmd.java:41)
    at ghidra.app.plugin.core.function.StackVariableAnalyzer.added(StackVariableAnalyzer.java:55)
    at ghidra.app.plugin.core.analysis.AnalysisScheduler.runAnalyzer(AnalysisScheduler.java:186)
    at ghidra.app.plugin.core.analysis.AnalysisTask.applyTo(AnalysisTask.java:37)
    at ghidra.app.plugin.core.analysis.AnalysisTask.applyTo(AnalysisTask.java:24)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisTaskWrapper.run(AutoAnalysisManager.java:660)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:760)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:639)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:604)
    at ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand.applyTo(AnalysisBackgroundCommand.java:55)
    at ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand.applyTo(AnalysisBackgroundCommand.java:33)
    at ghidra.framework.plugintool.mgr.ToolTaskManager.taskCompleted(ToolTaskManager.java:413)
    at ghidra.framework.plugintool.mgr.BackgroundCommandTask.run(BackgroundCommandTask.java:105)
    at ghidra.framework.plugintool.mgr.ToolTaskManager.run(ToolTaskManager.java:351)
    at java.base/java.lang.Thread.run(Thread.java:1583)

---------------------------------------------------
Build Date: 2024-Oct-17 0022 BST
Ghidra Version: 11.3
Java Home: /usr/lib/jvm/java-21-openjdk-amd64
JVM Version: Ubuntu 21.0.4
OS: Linux 6.8.0-45-generic amd64

I get a similar error when I try to add a reference in the References Editor:

Not a valid Address on instruction at 1305
ghidra.util.exception.AssertException: Not a valid Address on instruction at 1305
    at ghidra.program.util.SymbolicPropogator.applyPcode(SymbolicPropogator.java:1047)
    at ghidra.program.util.SymbolicPropogator.flowConstants(SymbolicPropogator.java:529)
    at ghidra.program.util.SymbolicPropogator.flowConstants(SymbolicPropogator.java:425)
    at ghidra.program.util.SymbolicPropogator.flowConstants(SymbolicPropogator.java:179)
    at ghidra.app.cmd.function.CallDepthChangeInfo.followFlows(CallDepthChangeInfo.java:565)
    at ghidra.app.cmd.function.CallDepthChangeInfo.initialize(CallDepthChangeInfo.java:154)
    at ghidra.app.cmd.function.CallDepthChangeInfo.<init>(CallDepthChangeInfo.java:82)
    at ghidra.app.plugin.core.references.EditStackReferencePanel.setOpIndex(EditStackReferencePanel.java:142)
    at ghidra.app.plugin.core.references.EditStackReferencePanel.initialize(EditStackReferencePanel.java:116)
    at ghidra.app.plugin.core.references.EditReferenceDialog.setAddOpIndex(EditReferenceDialog.java:192)
    at ghidra.app.plugin.core.references.EditReferenceDialog.configureAddReference(EditReferenceDialog.java:261)
    at ghidra.app.plugin.core.references.EditReferenceDialog.initDialog(EditReferenceDialog.java:249)
    at ghidra.app.plugin.core.references.ReferencesPlugin.popupAddReferenceDialog(ReferencesPlugin.java:303)
    at ghidra.app.plugin.core.references.EditReferencesProvider.addCallback(EditReferencesProvider.java:779)
    at ghidra.app.plugin.core.references.EditReferencesProvider$2.actionPerformed(EditReferencesProvider.java:167)
    at docking.DockingActionPerformer.lambda$perform$0(DockingActionPerformer.java:73)
    at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:318)
    at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:773)
    at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:720)
    at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:714)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
    at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:87)
    at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
    at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

---------------------------------------------------
Build Date: 2024-Oct-17 0022 BST
Ghidra Version: 11.3
Java Home: /usr/lib/jvm/java-21-openjdk-amd64
JVM Version: Ubuntu 21.0.4
OS: Linux 6.8.0-45-generic amd64