Skip to content

Commit 2c77971

Browse files
authored
Merge commit from fork
* Check return value of cs_vsnprintf for negative values. This prevents underflow of SStream.index. This bug was reported by Github user Finder16. * Add overflow check before adding cs_vsnprintf return value.
1 parent 9a0a160 commit 2c77971

File tree

4 files changed

+36
-2
lines changed

4 files changed

+36
-2
lines changed

SStream.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,10 @@ void SStream_concat(SStream *ss, const char *fmt, ...)
236236
ret = cs_vsnprintf(ss->buffer + ss->index,
237237
sizeof(ss->buffer) - (ss->index + 1), fmt, ap);
238238
va_end(ap);
239+
if (ret < 0) {
240+
return;
241+
}
242+
SSTREAM_OVERFLOW_CHECK(ss, ret);
239243
ss->index += ret;
240244
if (ss->markup_stream && ss->prefixed_by_markup) {
241245
SSTREAM_OVERFLOW_CHECK(ss, 1);

SStream.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ typedef enum {
1818

1919
typedef struct SStream {
2020
char buffer[SSTREAM_BUF_LEN];
21-
int index;
21+
size_t index;
2222
bool is_closed;
2323
bool markup_stream; ///< If true, markups to the stream are allowed.
2424
bool prefixed_by_markup; ///< Set after the stream wrote a markup for an operand.

tests/unit/include/unit_test.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
#define CHECK_INT_EQUAL_RET_FALSE(a, b) \
3939
do { \
4040
if (a != b) { \
41-
printf("%" PRId32 " != %" PRId32 "\n", a, b); \
41+
printf("%zu != %" PRId32 "\n", a, b); \
4242
return false; \
4343
} \
4444
} while (0);

tests/unit/sstream.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,35 @@ bool test_replc_str()
594594
return true;
595595
}
596596

597+
static int evil_vsnprintf(char *str, size_t size, const char *fmt, va_list ap)
598+
{
599+
(void)str;
600+
(void)size;
601+
(void)fmt;
602+
(void)ap;
603+
return -1; // forces index underflow
604+
}
605+
606+
/// Possible underflow of SStream.index and subsequent OOB reads/writes.
607+
/// Reported by Finder16.
608+
bool test_underflow_in_sstream(void)
609+
{
610+
cs_opt_mem mem = { .malloc = malloc,
611+
.calloc = calloc,
612+
.realloc = realloc,
613+
.free = free,
614+
.vsnprintf = evil_vsnprintf };
615+
cs_option(0, CS_OPT_MEM, (size_t)&mem);
616+
617+
SStream OS;
618+
SStream_Init(&OS);
619+
SStream_concat(&OS, "%s", "AAAA"); // index += -1
620+
SStream_concat1(&OS, 'B'); // writes before buffer => crash/ASan hit
621+
CHECK_OS_EQUAL_RET_FALSE(OS, "B");
622+
CHECK_INT_EQUAL_RET_FALSE(OS.index, 1);
623+
return true;
624+
}
625+
597626
int main()
598627
{
599628
bool result = true;
@@ -614,6 +643,7 @@ int main()
614643
result &= test_replc_str();
615644
result &= test_copy_mnem_opstr();
616645
result &= test_trimls();
646+
result &= test_underflow_in_sstream();
617647
if (result) {
618648
printf("All tests passed.\n");
619649
} else {

0 commit comments

Comments
 (0)