Skip to content

Commit

Permalink
Coverity defects (capstone-engine#2469)
Browse files Browse the repository at this point in the history
* Fix CID 508418 - Uninitialized struct

* Fix CID 509089 - Fix OOB read and write

* Fix CID 509088 - OOB.

Also adds tests and to ensure no OOB access.

* Fix CID 509085 - Resource leak.

* Fix CID 508414 and companions - Using undefined values.

* Fix CID 508405 - Use of uninitialized value

* Remove unnecessary and badly implemented dev fuzz code.

* Fix CID 508396 - Uninitialzied variable.

* Fix CID 508393, 508365 -- OOB read.

* Fix CID 432207 - OVerlapping memory access.

* Remove unused functions

* Fix CID 432170 - Overlapping memory access.

* Fix CID 166022 - Check for negative index

* Let strncat not depend n src operand.

* Fix 509083 and 509084 - NULL dereference

* Remove duplicated code.

* Initialize sysop

* Fix resource leak

* Remove unreachable code.

* Remove duplicate code.

* Add assert to check return value of cmoack

* Fixed: d should be a signed value, since it is checked against < 0

* Add missing break.

* Add NULL check

* Fix signs of binary search comparisons.

* Add explicit cast of or result

* Fix correct scope of case.

* Handle invalid integer type.

* Return UINT_MAX instead of implicitly casted -1

* Remove dead code

* Fix type of im

* Fix type of d

* Remove duplicated code.

* Add returns after CS_ASSERTS

* Check for len == 0 case.

* Ensure shift operates on uint64

* Replace strcpy with strncpy.

* Handle edge cases for 32bit rotate

* Fix some out of enum warnings

* Replace a strcpy with strncpy.

* Fix increment of address

* Skip some linting

* Fix: set instruction id

* Remove unused enum

* Replace the last usages of strcpy with SStream functions.

* Increase number of allowed AArch64 operands.

* Check safety of incrementing t the next operand.

* Fix naming of operand

* Update python constants

* Fix option setup of CS_OPT_DETAIL_REAL

* Document DETAIL_REAL has to be used with CS_OPT_ON.

* Run Coverity scan every Monday.

* Remove dead code

* Fix OOB read

* Rename macro to reflect it is only used with sstreams

* Fix rebase issues
  • Loading branch information
Rot127 authored Sep 18, 2024
1 parent af1ed2f commit 3a2cd3c
Show file tree
Hide file tree
Showing 70 changed files with 843 additions and 405 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coverity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Coverity Scan
on:
workflow_dispatch:
schedule:
- cron: '0 0 01 * *' # On the 1st every month at midnight UTC
- cron: '0 0 * * 1' # On every Monday at midnight UTC


# Automatically cancel any previous workflow on new push.
Expand Down
4 changes: 2 additions & 2 deletions MCInstPrinter.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ unsigned int binsearch_IndexTypeStrEncoding(const struct IndexTypeStr *index, si

right = size - 1;

size_t str_left_cmp = strcmp(name, index[0].name);
size_t str_right_cmp = strcmp(name, index[right].name);
int str_left_cmp = strcmp(name, index[0].name);
int str_right_cmp = strcmp(name, index[right].name);
if (str_left_cmp < 0 || str_right_cmp > 0)
// not found
return -1;
Expand Down
12 changes: 9 additions & 3 deletions Mapping.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ void map_implicit_reads(MCInst *MI, const insn_map *imap)
return;
}
detail->regs_read[detail->regs_read_count++] = reg;
reg = imap[Opcode].regs_use[++i];
if (i + 1 < MAX_IMPL_R_REGS) {
// Select next one
reg = imap[Opcode].regs_use[++i];
}
}
#endif // CAPSTONE_DIET
}
Expand All @@ -175,7 +178,10 @@ void map_implicit_writes(MCInst *MI, const insn_map *imap)
return;
}
detail->regs_write[detail->regs_write_count++] = reg;
reg = imap[Opcode].regs_mod[++i];
if (i + 1 < MAX_IMPL_W_REGS) {
// Select next one
reg = imap[Opcode].regs_mod[++i];
}
}
#endif // CAPSTONE_DIET
}
Expand Down Expand Up @@ -348,7 +354,7 @@ DEFINE_get_detail_op(systemz, SystemZ);
/// So it can be toggled between disas() calls.
bool map_use_alias_details(const MCInst *MI) {
assert(MI);
return !(MI->csh->detail_opt & CS_OPT_DETAIL_REAL);
return (MI->csh->detail_opt & CS_OPT_ON) && !(MI->csh->detail_opt & CS_OPT_DETAIL_REAL);
}

/// Sets the setDetailOps flag to @p Val.
Expand Down
16 changes: 16 additions & 0 deletions Mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,22 @@ DEFINE_get_arch_detail(mips, Mips);
DEFINE_get_arch_detail(riscv, RISCV);
DEFINE_get_arch_detail(systemz, SystemZ);

#define DEFINE_check_safe_inc(Arch, ARCH) \
static inline void Arch##_check_safe_inc() { \
CS_ASSERT(Arch##_get_detail(MI)->op_count + 1 < NUM_##ARCH##_OPS); \
}

DEFINE_check_safe_inc(ARM, ARM);
DEFINE_check_safe_inc(PPC, PPC);
DEFINE_check_safe_inc(TriCore, TRICORE);
DEFINE_check_safe_inc(AArch64, AARCH64);
DEFINE_check_safe_inc(Alpha, ALPHA);
DEFINE_check_safe_inc(HPPA, HPPA);
DEFINE_check_safe_inc(LoongArch, LOONGARCH);
DEFINE_check_safe_inc(RISCV, RISCV);
DEFINE_check_safe_inc(SystemZ, SYSTEMZ);
DEFINE_check_safe_inc(Mips, MIPS);

static inline bool detail_is_set(const MCInst *MI)
{
assert(MI && MI->flat_insn);
Expand Down
12 changes: 10 additions & 2 deletions MathExtras.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
#endif
#endif

#include <limits.h>

// NOTE: The following support functions use the _32/_64 extensions instead of
// type overloading so that signed and unsigned integers can be used without
// ambiguity.
Expand Down Expand Up @@ -280,15 +282,21 @@ static inline unsigned CountPopulation_64(uint64_t Value) {
}

/// Log2_32 - This function returns the floor log base 2 of the specified value,
/// -1 if the value is zero. (32 bit edition.)
/// UINT_MAX if the value is zero. (32 bit edition.)
/// Ex. Log2_32(32) == 5, Log2_32(1) == 0, Log2_32(0) == -1, Log2_32(6) == 2
static inline unsigned Log2_32(uint32_t Value) {
if (Value == 0) {
return UINT_MAX;
}
return 31 - CountLeadingZeros_32(Value);
}

/// Log2_64 - This function returns the floor log base 2 of the specified value,
/// -1 if the value is zero. (64 bit edition.)
/// UINT_MAX if the value is zero. (64 bit edition.)
static inline unsigned Log2_64(uint64_t Value) {
if (Value == 0) {
return UINT32_MAX;
}
return 63 - CountLeadingZeros_64(Value);
}

Expand Down
98 changes: 93 additions & 5 deletions SStream.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,108 @@
#include "cs_priv.h"
#include "utils.h"

#ifdef _MSC_VER
#pragma warning(disable: 4996) // disable MSVC's warning on strcpy()
#endif

void SStream_Init(SStream *ss)
{
assert(ss);
ss->index = 0;
ss->buffer[0] = '\0';
memset(ss->buffer, 0, sizeof(ss->buffer));
ss->is_closed = false;
ss->markup_stream = false;
ss->prefixed_by_markup = false;
}

/// Returns the a pointer to the internal string buffer of the stream.
/// For reading only.
const char *SStream_rbuf(const SStream *ss) {
assert(ss);
return ss->buffer;
}

/// Searches in the stream for the first (from the left) occurrence of @elem and replaces
/// it with @repl. It returns the pointer *after* the replaced character
/// or NULL if no character was replaced.
///
/// It will never replace the final \0 byte in the stream buffer.
const char *SStream_replc(const SStream *ss, char elem, char repl) {
assert(ss);
char *found = strchr(ss->buffer, elem);
if (!found || found == ss->buffer + (SSTREAM_BUF_LEN - 1)) {
return NULL;
}
*found = repl;
found++;
return found;
}

/// Searches in the stream for the first (from the left) occurrence of @chr and replaces
/// it with @rstr.
void SStream_replc_str(SStream *ss, char chr, const char *rstr) {
assert(ss && rstr);
char *found = strchr(ss->buffer, chr);
if (!found || found == ss->buffer + (SSTREAM_BUF_LEN - 1)) {
return;
}
size_t post_len = strlen(found + 1);
size_t buf_str_len = strlen(ss->buffer);
size_t repl_len = strlen(rstr);
if (repl_len - 1 + buf_str_len >= SSTREAM_BUF_LEN) {
return;
}
memmove(found + repl_len, found + 1, post_len);
memcpy(found, rstr, repl_len);
ss->index = strlen(ss->buffer);
}

/// Removes the space characters '\t' and ' ' from the beginning of the stream buffer.
void SStream_trimls(SStream *ss) {
assert(ss);
size_t buf_off = 0;
/// Remove leading spaces
while (ss->buffer[buf_off] == ' ' || ss->buffer[buf_off] == '\t') {
buf_off++;
}
if (buf_off > 0) {
memmove(ss->buffer, ss->buffer + buf_off, SSTREAM_BUF_LEN - buf_off);
ss->index -= buf_off;
}
}

/// Extract the mnemonic to @mnem_buf and the operand string into @op_str_buf from the stream buffer.
/// The mnemonic is everything up until the first ' ' or '\t' character.
/// The operand string is everything after the first ' ' or '\t' sequence.
void SStream_extract_mnem_opstr(const SStream *ss, char *mnem_buf, size_t mnem_buf_size, char *op_str_buf, size_t op_str_buf_size) {
assert(ss && mnem_buf && mnem_buf_size > 0 && op_str_buf && op_str_buf_size > 0);
size_t off = 0;
// Copy all non space chars to as mnemonic.
while (ss->buffer[off] && ss->buffer[off] != ' ' && ss->buffer[off] != '\t') {
if (off < mnem_buf_size - 1) {
// Only copy if there is space left.
mnem_buf[off] = ss->buffer[off];
}
off++;
}
if (!ss->buffer[off]) {
return;
}

// Iterate until next non space char.
do {
off++;
} while (ss->buffer[off] && (ss->buffer[off] == ' ' || ss->buffer[off] == '\t'));

if (!ss->buffer[off]) {
return;
}

// Copy all follow up characters as op_str
const char *ss_op_str = ss->buffer + off;
off = 0;
while (ss_op_str[off] && off < op_str_buf_size - 1) {
op_str_buf[off] = ss_op_str[off];
off++;
}
}

/// Empty the stream @ss to given @file (stdin/stderr).
/// @file can be NULL. Then the buffer content is not emitted.
void SStream_Flush(SStream *ss, FILE *file)
Expand Down
10 changes: 10 additions & 0 deletions SStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ do { \

void SStream_Init(SStream *ss);

const char *SStream_replc(const SStream *ss, char elem, char repl);

void SStream_replc_str(SStream *ss, char chr, const char *rstr);

const char *SStream_rbuf(const SStream *ss);

void SStream_extract_mnem_opstr(const SStream *ss, char *mnem_buf, size_t mnem_buf_size, char *op_str_buf, size_t op_str_buf_size);

void SStream_trimls(SStream *ss);

void SStream_Flush(SStream *ss, FILE *file);

void SStream_Open(SStream *ss);
Expand Down
5 changes: 4 additions & 1 deletion arch/AArch64/AArch64AddressingModes.h
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,10 @@ static inline uint64_t AArch64_AM_decodeLogicalImmediate(uint64_t val,
unsigned imms = val & 0x3f;

int len = 31 - countLeadingZeros((N << 6) | (~imms & 0x3f));
assert(len >= 1);
if (len < 1) {
assert(len >= 1 && "Unhandled integer type");
return 0;
}

unsigned size = (1 << len);
unsigned R = immr & (size - 1);
Expand Down
10 changes: 5 additions & 5 deletions arch/AArch64/AArch64InstPrinter.c
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ Search_IC: {
!AArch64_testFeatureList(MI->csh->mode, IC->FeaturesRequired))
return false;
if (detail_is_set(MI)) {
aarch64_sysop sysop;
aarch64_sysop sysop = { 0 };
sysop.reg = IC->SysReg;
sysop.sub_type = AARCH64_OP_IC;
AArch64_get_detail_op(MI, 0)->type = AARCH64_OP_SYSREG;
Expand All @@ -754,7 +754,7 @@ Search_IC: {
MI->csh->mode, DC->FeaturesRequired))
return false;
if (detail_is_set(MI)) {
aarch64_sysop sysop;
aarch64_sysop sysop = { 0 };
sysop.alias = DC->SysAlias;
sysop.sub_type = AARCH64_OP_DC;
AArch64_get_detail_op(MI, 0)->type =
Expand All @@ -777,7 +777,7 @@ Search_IC: {
return false;

if (detail_is_set(MI)) {
aarch64_sysop sysop;
aarch64_sysop sysop = { 0 };
sysop.alias = AT->SysAlias;
sysop.sub_type = AARCH64_OP_AT;
AArch64_get_detail_op(MI, 0)->type =
Expand All @@ -799,7 +799,7 @@ Search_IC: {
return false;

if (detail_is_set(MI)) {
aarch64_sysop sysop;
aarch64_sysop sysop = { 0 };
sysop.reg = TLBI->SysReg;
sysop.sub_type = AARCH64_OP_TLBI;
AArch64_get_detail_op(MI, 0)->type = AARCH64_OP_SYSREG;
Expand Down Expand Up @@ -868,7 +868,7 @@ bool printSyspAlias(MCInst *MI, SStream *O)
return false;

if (detail_is_set(MI)) {
aarch64_sysop sysop;
aarch64_sysop sysop = { 0 };
sysop.reg = TLBI->SysReg;
sysop.sub_type = AARCH64_OP_TLBI;
AArch64_get_detail_op(MI, 0)->type = AARCH64_OP_SYSREG;
Expand Down
Loading

0 comments on commit 3a2cd3c

Please sign in to comment.