From cf7f4d0e970d782577651b90c3813a292ef25bd8 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 26 Oct 2026 15:02:30 +0100 Subject: [PATCH] Fix UB in `cbor_value_get_int64()` with out-of-range inputs This function and `cbor_value_get_int()` are documented to "silently truncate" the values out of range and that untrusted input should be checked with their `_checked()` versions. However, it only truncated as a side-effect of UB, in the typical build. We cannot depend on that. The simplest fix is to just use unsigned arithmetic to do exactly what we were doing. The generated assembly will be exactly what we want and probably what was being generated anyway. Fixes #347. Signed-off-by: Thiago Macieira --- src/cbor.h | 12 ++++++++---- tests/parser/data.cpp | 8 ++++---- tests/parser/tst_parser.cpp | 15 ++++++++++----- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/cbor.h b/src/cbor.h index d5570265..89105951 100644 --- a/src/cbor.h +++ b/src/cbor.h @@ -431,19 +431,23 @@ CBOR_INLINE_API CborError cbor_value_get_uint64(const CborValue *value, uint64_t CBOR_INLINE_API CborError cbor_value_get_int64(const CborValue *value, int64_t *result) { + uint64_t v; assert(cbor_value_is_integer(value)); - *result = (int64_t) _cbor_value_extract_int64_helper(value); + v = _cbor_value_extract_int64_helper(value); if (value->flags & CborIteratorFlag_NegativeInteger) - *result = -*result - 1; + v = -v - 1; + *result = (int64_t) v; return CborNoError; } CBOR_INLINE_API CborError cbor_value_get_int(const CborValue *value, int *result) { + unsigned v; assert(cbor_value_is_integer(value)); - *result = (int) _cbor_value_extract_int64_helper(value); + v = (unsigned) _cbor_value_extract_int64_helper(value); if (value->flags & CborIteratorFlag_NegativeInteger) - *result = -*result - 1; + v = -v - 1; + *result = (int) v; return CborNoError; } diff --git a/tests/parser/data.cpp b/tests/parser/data.cpp index af923cb0..da783757 100644 --- a/tests/parser/data.cpp +++ b/tests/parser/data.cpp @@ -60,7 +60,7 @@ void addIntegers() << quint64(std::numeric_limits::max()) << std::numeric_limits::max() << false << true; QTest::newRow("UINT64_MAX") << raw("\x1b" "\xff\xff\xff\xff" "\xff\xff\xff\xff") - << std::numeric_limits::max() << qint64(-123456) << false << false; + << std::numeric_limits::max() << qint64(-1) << false << false; // negative integers QTest::newRow("-1") << raw("\x20") << Q_UINT64_C(0) << Q_INT64_C(-1) << true << true; @@ -81,11 +81,11 @@ void addIntegers() << quint64(std::numeric_limits::max()) << std::numeric_limits::min() << true << true; - QTest::newRow("INT64_MIN-1") << raw("\x3b\x80\0\0\0""\0\0\0\0") << Q_UINT64_C(9223372036854775808) << qint64(-123456) << true << false; + QTest::newRow("INT64_MIN-1") << raw("\x3b\x80\0\0\0""\0\0\0\0") << Q_UINT64_C(9223372036854775808) << qint64(9223372036854775807) << true << false; QTest::newRow("-UINT64_MAX") << raw("\x3b" "\xff\xff\xff\xff" "\xff\xff\xff\xfe") - << (std::numeric_limits::max() - 1) << qint64(-123456) << true << false; + << (std::numeric_limits::max() - 1) << qint64(1) << true << false; QTest::newRow("-UINT64_MAX+1") << raw("\x3b" "\xff\xff\xff\xff" "\xff\xff\xff\xff") - << std::numeric_limits::max() << qint64(-123456) << true << false; + << std::numeric_limits::max() << qint64(0) << true << false; } [[maybe_unused]] void addFloatingPoint() diff --git a/tests/parser/tst_parser.cpp b/tests/parser/tst_parser.cpp index 114899d2..dd3d3717 100644 --- a/tests/parser/tst_parser.cpp +++ b/tests/parser/tst_parser.cpp @@ -376,16 +376,21 @@ void tst_Parser::integers() QVERIFY(cbor_value_is_unsigned_integer(&w.first)); } + // even if this isn't in int64_t's range, these should just silently + // truncate with no UB int64_t value; - if (inInt64Range) { - cbor_value_get_int64(&w.first, &value); - QCOMPARE(qint64(value), expectedValue); - } + err = cbor_value_get_int64(&w.first, &value); + QCOMPARE(err, CborNoError); + QCOMPARE(qint64(value), expectedValue); + int ivalue; + err = cbor_value_get_int(&w.first, &ivalue); + QCOMPARE(err, CborNoError); + QCOMPARE(qint64(value), expectedValue); + // but these check err = cbor_value_get_int64_checked(&w.first, &value); QCOMPARE(err, inInt64Range ? CborNoError : CborErrorDataTooLarge); - int ivalue; bool inIntRange = inInt64Range && (expectedValue == int(expectedValue)); err = cbor_value_get_int_checked(&w.first, &ivalue); QCOMPARE(err, inIntRange ? CborNoError : CborErrorDataTooLarge);