diff --git a/CHANGELOG.md b/CHANGELOG.md index a0237927f..720d542d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - `sentry_set_trace` omits `parent_span_id` when the caller does not provide one, instead of serializing it as `null`. ([#2047](https://github.com/getsentry/sentry-native/pull/2047)) - Native/Linux i386: write valid thread stack descriptors to minidumps when stack addresses use the upper half of the 32-bit address space. ([#2054](https://github.com/getsentry/sentry-native/pull/2054)) - Guard size arithmetic when parsing envelopes and Linux OS release data, copying slices, and allocating memory during crash handling. ([#2059](https://github.com/getsentry/sentry-native/pull/2059)) +- Prevent out-of-bounds reads when parsing JSON numbers from length-delimited buffers. ([#2067](https://github.com/getsentry/sentry-native/pull/2067)) **Thank you**: diff --git a/src/sentry_json.c b/src/sentry_json.c index f9e3352cc..08ecf6dfd 100644 --- a/src/sentry_json.c +++ b/src/sentry_json.c @@ -17,6 +17,7 @@ #include "sentry_alloc.h" #include "sentry_core.h" #include "sentry_json.h" +#include "sentry_slice.h" #include "sentry_string.h" #include "sentry_utils.h" #include "sentry_value.h" @@ -536,6 +537,64 @@ decode_string_inplace(char *buf, size_t *len_out) return true; } +static bool +parse_number(sentry_slice_t slice, sentry_value_t *value_out) +{ + char number_buf[32]; + char *number = number_buf; + if (slice.len < sizeof(number_buf)) { + // stack buffer is sufficient for any SDK-supplied numbers + sentry__slice_to_buffer(slice, number_buf, sizeof(number_buf)); + } else { + // heap fallback for longer external numbers + number = sentry__slice_to_owned(slice); + if (!number) { + return false; + } + } + + bool success = false; + char *endptr = NULL; + sentry_value_t rv = sentry_value_new_null(); + errno = 0; + + if (number[0] == '-') { + const long long ll_val = strtoll(number, &endptr, 10); + if (endptr == number + slice.len && errno == 0) { + if (ll_val >= INT32_MIN && ll_val <= INT32_MAX) { + rv = sentry_value_new_int32((int32_t)ll_val); + } else { + rv = sentry_value_new_int64((int64_t)ll_val); + } + success = true; + } + } else { + const unsigned long long ull_val = strtoull(number, &endptr, 10); + if (endptr == number + slice.len && errno == 0) { + if (ull_val <= INT32_MAX) { + rv = sentry_value_new_int32((int32_t)ull_val); + } else if (ull_val <= INT64_MAX) { + rv = sentry_value_new_int64((int64_t)ull_val); + } else { + rv = sentry_value_new_uint64((uint64_t)ull_val); + } + success = true; + } + } + + // Both failed, fallback to double + if (!success) { + double val = sentry__strtod_c(number, NULL); + rv = sentry_value_new_double(val); + } + + if (number != number_buf) { + sentry_free(number); + } + *value_out = rv; + return true; +} + static size_t tokens_to_value(jsmntok_t *tokens, size_t token_count, const char *buf, size_t depth, sentry_value_t *value_out) @@ -573,40 +632,10 @@ tokens_to_value(jsmntok_t *tokens, size_t token_count, const char *buf, rv = sentry_value_new_null(); break; default: { - bool success = false; - char *endptr; - errno = 0; - - if (buf[root->start] == '-') { - const long long ll_val - = strtoll(buf + root->start, &endptr, 10); - if (endptr == buf + root->end && errno == 0) { - if (ll_val >= INT32_MIN && ll_val <= INT32_MAX) { - rv = sentry_value_new_int32((int32_t)ll_val); - } else { - rv = sentry_value_new_int64((int64_t)ll_val); - } - success = true; - } - } else { - const unsigned long long ull_val - = strtoull(buf + root->start, &endptr, 10); - if (endptr == buf + root->end && errno == 0) { - if (ull_val <= INT32_MAX) { - rv = sentry_value_new_int32((int32_t)ull_val); - } else if (ull_val <= INT64_MAX) { - rv = sentry_value_new_int64((int64_t)ull_val); - } else { - rv = sentry_value_new_uint64((uint64_t)ull_val); - } - success = true; - } - } - - // Both failed, fallback to double - if (!success) { - double val = sentry__strtod_c(buf + root->start, NULL); - rv = sentry_value_new_double(val); + sentry_slice_t slice + = { buf + root->start, (size_t)(root->end - root->start) }; + if (!parse_number(slice, &rv)) { + goto error; } break; } diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 228883c12..d0b8ea611 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -1191,6 +1191,16 @@ SENTRY_TEST(value_json_parsing) TEST_CHECK_INT_EQUAL(sentry_value_as_int32(rv), 42); sentry_value_decref(rv); + const char number_with_trailing_digit[] = "420"; + rv = sentry__value_from_json(number_with_trailing_digit, 2); + TEST_CHECK(sentry_value_get_type(rv) == SENTRY_VALUE_TYPE_INT32); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(rv), 42); + sentry_value_decref(rv); + + rv = sentry__value_from_json(STRING("11111111111111111111111111111111")); + TEST_CHECK(sentry_value_get_type(rv) == SENTRY_VALUE_TYPE_DOUBLE); + sentry_value_decref(rv); + rv = sentry__value_from_json(STRING("-9223372036854775808")); TEST_CHECK(sentry_value_get_type(rv) == SENTRY_VALUE_TYPE_INT64); TEST_CHECK_INT_EQUAL(sentry_value_as_int64(rv), INT64_MIN);