Skip to content

Commit

Permalink
Fixing UB santizer, LITBASE and assert errors. (capstone-engine#2499)
Browse files Browse the repository at this point in the history
  • Loading branch information
Rot127 authored Oct 6, 2024
1 parent 5bd05e3 commit 52b54ee
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 40 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/CITest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ concurrency:

env:
CI: true
UBSAN_OPTIONS: "halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1"
ASAN_OPTIONS: "halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1"
LSAN_OPTIONS: "halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1"

jobs:
Linux:
Expand Down Expand Up @@ -86,10 +89,10 @@ jobs:
mkdir build && cd build
# build static library
cmake -DCAPSTONE_INSTALL=1 -DCMAKE_INSTALL_PREFIX=/usr -DENABLE_ASAN=${asan} -DCAPSTONE_BUILD_DIET=${diet_build} ..
cmake --build . --config Release
cmake --build . --config Debug
# build shared library
cmake -DCAPSTONE_INSTALL=1 -DBUILD_SHARED_LIBS=1 -DCMAKE_INSTALL_PREFIX=/usr -DCAPSTONE_BUILD_CSTEST=ON -DENABLE_ASAN=${asan} ..
sudo cmake --build . --config Release --target install
sudo cmake --build . --config Debug --target install
- name: Lower number of KASL randomized address bits
run: |
Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ option(ENABLE_COVERAGE "Enable test coverage" OFF)
if (ENABLE_ASAN)
message("Enabling ASAN")
add_definitions(-DASAN_ENABLED)
add_compile_options(-fsanitize=address)
add_link_options(-fsanitize=address)
add_compile_options(-fsanitize=address,undefined)
add_link_options(-fsanitize=address,undefined)
endif()

if (ENABLE_COVERAGE)
Expand Down
5 changes: 4 additions & 1 deletion SStream.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,10 @@ void printInt32(SStream *O, int32_t val)
SStream_concat(O, "%" PRId32, val);
} else {
if (val < -HEX_THRESHOLD) {
SStream_concat(O, "-0x%" PRIx32, (uint32_t)-val);
if (val == INT32_MIN)
SStream_concat(O, "-0x%" PRIx32, (uint32_t) INT32_MAX + 1);
else
SStream_concat(O, "-0x%" PRIx32, (int32_t)-val);
} else {
SStream_concat(O, "-%" PRIu32, (uint32_t)-val);
}
Expand Down
6 changes: 3 additions & 3 deletions arch/AArch64/AArch64AddressingModes.h
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,9 @@ static inline float AArch64_AM_getFPImmFloat(unsigned Imm)
{
// We expect an 8-bit binary encoding of a floating-point number here.

uint8_t Sign = (Imm >> 7) & 0x1;
uint8_t Exp = (Imm >> 4) & 0x7;
uint8_t Mantissa = Imm & 0xf;
uint32_t Sign = (Imm >> 7) & 0x1;
uint32_t Exp = (Imm >> 4) & 0x7;
uint32_t Mantissa = Imm & 0xf;

// 8-bit FP IEEE Float Encoding
// abcd efgh aBbbbbbc defgh000 00000000 00000000
Expand Down
6 changes: 3 additions & 3 deletions arch/ARM/ARMAddressingModes.h
Original file line number Diff line number Diff line change
Expand Up @@ -769,9 +769,9 @@ static inline float ARM_AM_getFPImmFloat(unsigned Imm)
{
// We expect an 8-bit binary encoding of a floating-point number here.

uint8_t Sign = (Imm >> 7) & 0x1;
uint8_t Exp = (Imm >> 4) & 0x7;
uint8_t Mantissa = Imm & 0xf;
uint32_t Sign = (Imm >> 7) & 0x1;
uint32_t Exp = (Imm >> 4) & 0x7;
uint32_t Mantissa = Imm & 0xf;

// 8-bit FP IEEE Float Encoding
// abcd efgh aBbbbbbc defgh000 00000000 00000000
Expand Down
16 changes: 8 additions & 8 deletions arch/HPPA/HPPADisassembler.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ static int extract_16(unsigned word, bool wide)

/* Extract a 21 bit constant. */

static int extract_21(unsigned word)
static int32_t extract_21(unsigned word)
{
int val;

Expand All @@ -177,14 +177,14 @@ static int extract_21(unsigned word)
val |= get_insn_field(word, 0, 4);
val <<= 2;
val |= get_insn_field(word, 7, 8);
return SignExtend32(val, 21) << 11;
return (uint32_t) SignExtend32(val, 21) << 11;
}

/* Extract a 12 bit constant from branch instructions. */

static int extract_12(unsigned word)
static int32_t extract_12(unsigned word)
{
return SignExtend32(get_insn_field(word, 19, 28) |
return (uint32_t) SignExtend32(get_insn_field(word, 19, 28) |
get_insn_field(word, 29, 29) << 10 |
(word & 0x1) << 11,
12)
Expand All @@ -194,19 +194,19 @@ static int extract_12(unsigned word)
/* Extract a 17 bit constant from branch instructions, returning the
19 bit signed value. */

static int extract_17(unsigned word)
static int32_t extract_17(unsigned word)
{
return SignExtend32(get_insn_field(word, 19, 28) |
return (uint32_t) SignExtend32(get_insn_field(word, 19, 28) |
get_insn_field(word, 29, 29) << 10 |
get_insn_field(word, 11, 15) << 11 |
(word & 0x1) << 16,
17)
<< 2;
}

static int extract_22(unsigned word)
static int32_t extract_22(unsigned word)
{
return SignExtend32(get_insn_field(word, 19, 28) |
return (uint32_t) SignExtend32(get_insn_field(word, 19, 28) |
get_insn_field(word, 29, 29) << 10 |
get_insn_field(word, 11, 15) << 11 |
get_insn_field(word, 6, 10) << 16 |
Expand Down
4 changes: 2 additions & 2 deletions arch/Xtensa/XtensaDisassembler.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ static DecodeStatus decodeImm8Operand(MCInst *Inst, uint64_t Imm,
static DecodeStatus decodeImm8_sh8Operand(MCInst *Inst, uint64_t Imm,
int64_t Address, const void *Decoder)
{
CS_ASSERT(isUIntN(8, Imm) && "Invalid immediate");
MCOperand_CreateImm0(Inst, (SignExtend64((Imm << 8), 16)));
CS_ASSERT(isUIntN(16, Imm) && "Invalid immediate");
MCOperand_CreateImm0(Inst, (SignExtend64((Imm), 16)));
return MCDisassembler_Success;
}

Expand Down
6 changes: 3 additions & 3 deletions arch/Xtensa/XtensaInstPrinter.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,13 @@ static inline void printL32RTarget(MCInst *MI, int OpNum, SStream *O)
if (MCOperand_isImm(MC)) {
int64_t Value =
MCOperand_getImm(MCInst_getOperand(MI, (OpNum)));
int64_t InstrOff = OneExtend32(Value << 2, 14);
int32_t InstrOff = (uint32_t)OneExtend32(Value, 16) << 2;
CS_ASSERT(
(Value >= -262144 && Value <= -4) &&
(InstrOff >= -262144 && InstrOff <= -4) &&
"Invalid argument, value must be in ranges [-262144,-4]");
SStream_concat0(O, ". ");
if (MI->csh->LITBASE & 0x1) {
Value = (int64_t)(MI->csh->LITBASE & 0x7ff) + InstrOff;
Value = ((MI->csh->LITBASE & 0xfffff000) >> 12) + InstrOff;
} else {
Value = (((int64_t)MI->address + 3) & ~0x3) + InstrOff;
}
Expand Down
8 changes: 4 additions & 4 deletions arch/Xtensa/XtensaMapping.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,12 @@ void Xtensa_add_cs_detail(MCInst *MI, xtensa_op_group op_group, va_list args)
case XTENSA_OP_GROUP_L32RTARGET: {
int64_t Value =
MCOperand_getImm(MCInst_getOperand(MI, (op_num)));
int64_t InstrOff = OneExtend32(Value << 2, 14);
int32_t InstrOff = (uint32_t)OneExtend32(Value, 16) << 2;
CS_ASSERT(
(Value >= -262144 && Value <= -4) &&
(InstrOff >= -262144 && InstrOff <= -4) &&
"Invalid argument, value must be in ranges [-262144,-4]");
if (MI->csh->LITBASE & 0x1) {
Value = (int64_t)(MI->csh->LITBASE & 0x7ff) + InstrOff;
Value = ((MI->csh->LITBASE & 0xfffff000) >> 12) + InstrOff;
} else {
Value = (((int64_t)MI->address + 3) & ~0x3) + InstrOff;
}
Expand All @@ -226,4 +226,4 @@ void Xtensa_add_cs_detail(MCInst *MI, xtensa_op_group op_group, va_list args)

xop->access = map_get_op_access(MI, op_num);
Xtensa_inc_op_count(MI);
}
}
2 changes: 1 addition & 1 deletion cs_priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ struct cs_struct {
const uint8_t *regsize_map; // map to register size (x86-only for now)
GetRegisterAccess_t reg_access;
struct insn_mnem *mnem_list; // linked list of customized instruction mnemonic
uint32_t LITBASE;
uint32_t LITBASE; ///< The LITBASE register content. Bit 0 (LSB) indicatess if it is set. Bit[23:8] are the literal base address.
};

#define MAX_ARCH CS_ARCH_MAX
Expand Down
2 changes: 1 addition & 1 deletion tests/MC/Xtensa/arith.s.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ test_cases:
options: [ "xtensa" ]
expected:
insns:
- asm_text: "addmi a1, a2, 0"
- asm_text: "addmi a1, a2, 0x7f00"

-
input:
Expand Down
20 changes: 10 additions & 10 deletions tests/integration/test_litbase.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,23 @@ static void test()
size_t count = 0;

count = cs_disasm(handle, (const uint8_t *)DATA, sizeof(DATA) - 1,
0x10000, 2, &insn);
0x100000, 2, &insn);

// 1. Print out the instruction in default setup.
printf("Disassemble xtensa code with PC=0x10000\n");
check_insn(insn, "l32r", "a1, . 0xc000");
check_insn(insn + 1, "l32r", "a1, . 0x10000");
printf("Disassemble xtensa code with PC=0x100000\n");
check_insn(insn, "l32r", "a1, . 0xc0000");
check_insn(insn + 1, "l32r", "a1, . 0x100000");
print_insn(insn, count);

// Customized mnemonic JNE to JNZ using CS_OPT_LITBASE option
printf("\nNow customize engine to change LITBASA to 0xff001\n");
cs_option(handle, CS_OPT_LITBASE, (size_t)0xff001);
printf("\nNow customize engine to change LITBASE to 0xfffff001\n");
cs_option(handle, CS_OPT_LITBASE, (size_t)0xfffff001);
count = cs_disasm(handle, (const uint8_t *)DATA, sizeof(DATA) - 1,
0x10000, 2, &insn);
0x100000, 2, &insn);

// 2. Now print out the instruction in newly customized setup.
check_insn(insn, "l32r", "a1, . -0x3fff");
check_insn(insn + 1, "l32r", "a1, . -3");
check_insn(insn, "l32r", "a1, . 0xbffff");
check_insn(insn + 1, "l32r", "a1, . 0xffffb");
print_insn(insn, count);

// Done
Expand All @@ -87,4 +87,4 @@ int main()
test();

return 0;
}
}

0 comments on commit 52b54ee

Please sign in to comment.