Skip to content

riscv: propagate register-class decode failure in custom decoders #3071

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.

Sign up for GitHub

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

Open
jmestwa-coder wants to merge 1 commit into
base: next
Choose a base branch
from
+120 −13
Open

riscv: propagate register-class decode failure in custom decoders #3071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 30 additions & 13 deletions arch/RISCV/RISCVDisassembler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,10 +438,9 @@ static DecodeStatus decodeRVCInstrRdRs1ImmZero(MCInst *Inst, uint32_t Insn,
const void *Decoder)
{
uint32_t Rd = fieldFromInstruction_4(Insn, 7, 5);
DecodeStatus Result =
DecodeGPRNoX0RegisterClass(Inst, Rd, Address, Decoder);
(void)Result;
CS_ASSERT(Result == MCDisassembler_Success && "Invalid register");
if (DecodeGPRNoX0RegisterClass(Inst, Rd, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
MCInst_addOperand2(Inst, (MCInst_getOperand(Inst, (0))));
MCOperand_CreateImm0(Inst, (0));
return MCDisassembler_Success;
Expand Down Expand Up @@ -491,8 +490,12 @@ static DecodeStatus decodeRVCInstrRdRs2(MCInst *Inst, uint32_t Insn,
{
uint32_t Rd = fieldFromInstruction_4(Insn, 7, 5);
uint32_t Rs2 = fieldFromInstruction_4(Insn, 2, 5);
DecodeGPRRegisterClass(Inst, Rd, Address, Decoder);
DecodeGPRRegisterClass(Inst, Rs2, Address, Decoder);
if (DecodeGPRRegisterClass(Inst, Rd, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
if (DecodeGPRRegisterClass(Inst, Rs2, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
return MCDisassembler_Success;
}

Expand All @@ -502,9 +505,13 @@ static DecodeStatus decodeRVCInstrRdRs1Rs2(MCInst *Inst, uint32_t Insn,
{
uint32_t Rd = fieldFromInstruction_4(Insn, 7, 5);
uint32_t Rs2 = fieldFromInstruction_4(Insn, 2, 5);
DecodeGPRRegisterClass(Inst, Rd, Address, Decoder);
if (DecodeGPRRegisterClass(Inst, Rd, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
MCInst_addOperand2(Inst, (MCInst_getOperand(Inst, (0))));
DecodeGPRRegisterClass(Inst, Rs2, Address, Decoder);
if (DecodeGPRRegisterClass(Inst, Rs2, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
return MCDisassembler_Success;
}

Expand All @@ -515,9 +522,15 @@ static DecodeStatus decodeXTHeadMemPair(MCInst *Inst, uint32_t Insn,
uint32_t Rs1 = fieldFromInstruction_4(Insn, 15, 5);
uint32_t Rd2 = fieldFromInstruction_4(Insn, 20, 5);
uint32_t UImm2 = fieldFromInstruction_4(Insn, 25, 2);
DecodeGPRRegisterClass(Inst, Rd1, Address, Decoder);
DecodeGPRRegisterClass(Inst, Rd2, Address, Decoder);
DecodeGPRRegisterClass(Inst, Rs1, Address, Decoder);
if (DecodeGPRRegisterClass(Inst, Rd1, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
if (DecodeGPRRegisterClass(Inst, Rd2, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
if (DecodeGPRRegisterClass(Inst, Rs1, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
DecodeStatus Result =
CONCAT(decodeUImmOperand, 2)(Inst, UImm2, Address, Decoder);
(void)Result;
Expand Down Expand Up @@ -549,8 +562,12 @@ static DecodeStatus decodeRegReg(MCInst *Inst, uint32_t Insn, uint64_t Address,
{
uint32_t Rs1 = fieldFromInstruction_4(Insn, 0, 5);
uint32_t Rs2 = fieldFromInstruction_4(Insn, 5, 5);
DecodeGPRRegisterClass(Inst, Rs1, Address, Decoder);
DecodeGPRRegisterClass(Inst, Rs2, Address, Decoder);
if (DecodeGPRRegisterClass(Inst, Rs1, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
if (DecodeGPRRegisterClass(Inst, Rs2, Address, Decoder) ==
MCDisassembler_Fail)
return MCDisassembler_Fail;
return MCDisassembler_Success;
}

Expand Down
53 changes: 53 additions & 0 deletions tests/integration/test_poc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,58 @@ int test_evil_vsnprintf_ghsa_gj26_93q5_cr54(void)
return 0;
}

/// Several hand-written RISC-V decoders dropped the status of
/// Decode*RegisterClass. In RVE mode (and the COREV/XTHead custom decoders) a
/// register field can be rejected by the class decoder, which then leaves the
/// operand uncreated while the instruction still decodes as "success". The
/// printer later reads that missing operand: printRegReg passes a NULL register
/// name to SStream_concat0 (NULL deref in release), and printOperand hits a
/// slot of unknown kind. Each word below reached one of the fixed decoders.
static void test_riscv_rve_unchecked_reg_decode(void)
{
static const struct {
cs_mode mode;
uint8_t code[4];
size_t size;
} cases[] = {
/* decodeRegReg: COREV cv.sh with an out-of-range rs */
{ CS_MODE_RISCV32 | CS_MODE_RISCV_E | CS_MODE_RISCV_COREV,
{ 0x2b, 0x33, 0x0a, 0x2a },
4 },
/* decodeXTHeadMemPair: th.lwd with rd2 = x17 */
{ CS_MODE_RISCV32 | CS_MODE_RISCV_E | CS_MODE_RISCV_THEAD,
{ 0x0b, 0x40, 0x11, 0xe1 },
4 },
/* decodeRVCInstrRdRs2 / decodeRVCInstrRdRs1Rs2 */
{ CS_MODE_RISCV32 | CS_MODE_RISCV_E | CS_MODE_RISCV_C,
{ 0x46, 0x80 },
2 },
{ CS_MODE_RISCV32 | CS_MODE_RISCV_E | CS_MODE_RISCV_C,
{ 0x46, 0x90 },
2 },
/* decodeRVCInstrRdRs1ImmZero */
{ CS_MODE_RISCV32 | CS_MODE_RISCV_E | CS_MODE_RISCV_C,
{ 0x01, 0x08 },
2 },
};

for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
csh handle;
if (cs_open(CS_ARCH_RISCV, cases[i].mode, &handle) !=
CS_ERR_OK) {
assert(0);
return;
}
cs_option(handle, CS_OPT_DETAIL, CS_OPT_ON);

cs_insn *insn = NULL;
size_t count = cs_disasm(handle, cases[i].code, cases[i].size,
0x1000, 0, &insn);
cs_free(insn, count);
cs_close(&handle);
}
}

int main()
{
test_overflow_cs_insn_bytes();
Expand All @@ -369,6 +421,7 @@ int main()
test_arm_pop_ghsa_8qp8_2vg2_8mr4();
test_tms320_ghsa_8qp8_2vg2_8mr4();
test_evil_vsnprintf_ghsa_gj26_93q5_cr54();
test_riscv_rve_unchecked_reg_decode();

return 0;
}
37 changes: 37 additions & 0 deletions tests/issues/fuzzing.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,40 @@ test_cases:
address: 0x0
expected:
insns: []

-
input:
name: "riscv: RVE COREV cv.sh with out-of-range reg must fail (decodeRegReg)"
bytes: [ 0x2b, 0x33, 0x0a, 0x2a ]
arch: "CS_ARCH_RISCV"
options: [ CS_MODE_RISCV32, CS_MODE_RISCV_E, CS_MODE_RISCV_XCVMEM, CS_OPT_DETAIL ]
address: 0x0
expected:
insns: []
-
input:
name: "riscv: RVE c.add with out-of-range reg must fail (decodeRVCInstrRdRs1Rs2)"
bytes: [ 0x46, 0x80 ]
arch: "CS_ARCH_RISCV"
options: [ CS_MODE_RISCV32, CS_MODE_RISCV_E, CS_MODE_RISCV_C, CS_OPT_DETAIL ]
address: 0x0
expected:
insns: []
-
input:
name: "riscv: RVE c.mv with out-of-range reg must fail (decodeRVCInstrRdRs2)"
bytes: [ 0x46, 0x90 ]
arch: "CS_ARCH_RISCV"
options: [ CS_MODE_RISCV32, CS_MODE_RISCV_E, CS_MODE_RISCV_C, CS_OPT_DETAIL ]
address: 0x0
expected:
insns: []
-
input:
name: "riscv: RVE compressed rd0-form with out-of-range reg must fail (decodeRVCInstrRdRs1ImmZero)"
bytes: [ 0x01, 0x08 ]
arch: "CS_ARCH_RISCV"
options: [ CS_MODE_RISCV32, CS_MODE_RISCV_E, CS_MODE_RISCV_C, CS_OPT_DETAIL ]
address: 0x0
expected:
insns: []
Loading