NationalSecurityAgency / ghidra

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

Sleigh NPE 'with blocks' with actions section #1158

Open astrelsky opened 4 years ago

astrelsky commented 4 years ago

Describe the bug If an instruction inside a with block does not have an action section and the with block does, a NullPointerException occurs.

with : epsilon [ dummyContext=1; ] {
   :dummy is epsilon {}
}

Environment (please complete the following information):

Additional context I traced it back to SleighCompiler.g line 601. b needs to be created if nil before the call to sc.pushWith but I'm not familiar with ANTLR so I'm unsure if it needs to happen before entering the withblock or just before calling pushWith.

mumbel commented 4 years ago

#745 is a band-aid solution

astrelsky commented 4 years ago

745 is a band-aid solution

I don't see how. The with block is trying to append its action section to null which is causing the exception here. This is actually what I am attempting to do.

define token instr(64)
    vucI=(63,63)
    vucI=(62,62)
    vucI=(61,61)
    vucI=(60,60)
    vucI=(59,59)
;

define register offset=0x350 size=4 contextreg;
define context contextreg
    microMode=(0,0)
    cI=(1,1)
    cE=(2,2)
    cM=(3,3)
    cD=(4,4)
    cT=(5,5)
;

with : microMode=1 & vucI & vucE & vucM & vucD & vucT [cI=vucI; cE=vucE; cM=vucM; cD=vucD; cT=vucT;] {
    # pretend the ops are properly defined
    inst1: ops1 is cE=0 & ops2 {}
    loi: IMM_FLOAT is cI=1 {} # that one is just an easy example. Load immediate float where the instruction bytes are the immediate float.
    inst3: ops3 is ops3 {}
    inst4: ops4 is ops4 {}
    inst5: ops5 is ops5 {}
    inst6: ops6 is ops6 {}
}

So since every instructions upper 5 bits have an effect on the next instruction (actually the prior one, thanks little endianess) it makes sense to do this. It would work fine if each one had an action section with something in it, but since they don't the with block tried to append cI=vucI; cE=vucE; cM=vucM; cD=vucD; cT=vucT; to null.

It is quite difficult to give examples for sleigh bugs due to the work involved to duplicate them. It may be a good idea to have a dummy processor set up for things of this nature.

mumbel commented 4 years ago

yeah, sorry just zoned in on action and figured it was the same bug, so simple to come across. btw stack trace would probably help the devs. Also Processors/Toy is pretty much what you're asking for in your last sentence.

https://github.com/NationalSecurityAgency/ghidra/blob/d081700b0c834a310dcfc0af1a9b1a8e3dfe7bdf/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcodeCPort/slgh_compile/SleighCompile.java#L213

an attempt to repro led to that contvec being null

astrelsky commented 4 years ago

yeah, sorry just zoned in on action and figured it was the same bug, so simple to come across. btw stack trace would probably help the devs. Also Processors/Toy is pretty much what you're asking for in your last sentence.

https://github.com/NationalSecurityAgency/ghidra/blob/d081700b0c834a310dcfc0af1a9b1a8e3dfe7bdf/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcodeCPort/slgh_compile/SleighCompile.java#L213

an attempt to repro led to that contvec being null

Yes but the root cause is further back. I didn't realize you could directly link to the line. https://github.com/NationalSecurityAgency/ghidra/blob/d081700b0c834a310dcfc0af1a9b1a8e3dfe7bdf/Ghidra/Framework/SoftwareModeling/src/main/antlr/ghidra/sleigh/grammar/SleighCompiler.g#L601 nil is being passed in as b. I don't know how to trace any further than that since it's a grammar file.

At one point I had a feeling that the Toy processor was for that purpose, but then I discovered it was a real processor. TOY

mumbel commented 4 years ago

Unfortunate name collision looks like, they don't have similar design/specs. There are also names that suggest it is example of sleigh/pcode (define pcodeop pcodeop_one;)

SamuelWAnderson45 commented 4 years ago

Found a similar NPE related to operands.

with instruction : op0=1 & reg1 {
    :EXMPL reg1 is op1=0 {
    }
}

crashes with

ERROR Unrecoverable error(s), halting compilation (SleighCompile) java.lang.NullPointerException
    at ghidra.pcodeCPort.slgh_compile.SleighCompile.defineInvisibleOperand(SleighCompile.java:1159)
    at ghidra.sleigh.grammar.SleighCompiler.pequation_symbol(SleighCompiler.java:4458)
    at ghidra.sleigh.grammar.SleighCompiler.pequation(SleighCompiler.java:4293)
    at ghidra.sleigh.grammar.SleighCompiler.pequation(SleighCompiler.java:4137)
    at ghidra.sleigh.grammar.SleighCompiler.bitpattern(SleighCompiler.java:3562)
    at ghidra.sleigh.grammar.SleighCompiler.bitpat_or_nil(SleighCompiler.java:3301)
    at ghidra.sleigh.grammar.SleighCompiler.withblock(SleighCompiler.java:3168)
    at ghidra.sleigh.grammar.SleighCompiler.constructorlike(SleighCompiler.java:2963)
    at ghidra.sleigh.grammar.SleighCompiler.root(SleighCompiler.java:481)
    at ghidra.pcodeCPort.slgh_compile.SleighCompileLauncher.run_compilation(SleighCompileLauncher.java:373)
    at ghidra.pcodeCPort.slgh_compile.SleighCompileLauncher.runMain(SleighCompileLauncher.java:277)
    at ghidra.pcodeCPort.slgh_compile.SleighCompileLauncher.launch(SleighCompileLauncher.java:74)
    at ghidra.GhidraLauncher.main(GhidraLauncher.java:79)

Moving reg1 to the constructor like so

with instruction : op0=1 {
    :EXMPL reg1 is op1=0 & reg1 {
    }
}

allows the compilation to finish. After looking at the Sleigh spec, its unclear to me whether operands belong in the with block. Even in that case, the compilation should fail with a proper error, not an NPE.