Merge pull request #367 from silicon-heaven/ub-fixes

Ub fixes
silicon-heaven · Oct 24, 2023 · b0f19e6 · b0f19e6
2 parents 0789e5e + 49b7666
commit b0f19e6
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 18 deletions.
diff --git a/.github/actions/run-linter/action.yml b/.github/actions/run-linter/action.yml
@@ -28,9 +28,9 @@ runs:
       run: |
         BASE_REF="${{github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before}}"
         git fetch --depth=1 origin "$BASE_REF"
-        # FIXME: The clang on CI chokes on lambda captures of structured binding variables. Remove the blacklist of
-        # test_serialportsocket after the CI gets updated.
-        readarray -t CHANGED_FILES < <(git diff --name-only "$BASE_REF" | grep 'cpp$' | grep -v "libshviotqt/tests/serialportsocket/test_serialportsocket.cpp")
+        # FIXME: The clang on CI chokes on lambda captures of structured binding variables. Remove the blacklisting of
+        # the files after the CI gets updated. (AFAIK clang-16).
+        readarray -t CHANGED_FILES < <(git diff --name-only "$BASE_REF" | grep 'cpp$' | grep -v -e "libshviotqt/tests/serialportsocket/test_serialportsocket.cpp" -e "libshvchainpack/tests/test_rpcvalue.cpp")
         if [[ "${#CHANGED_FILES[@]}" -eq 0 ]]; then
           echo "No changed cpp files."
           exit 0

diff --git a/libshvchainpack/c/cchainpack.c b/libshvchainpack/c/cchainpack.c
@@ -463,6 +463,11 @@ static void unpack_int(ccpcp_unpack_context* unpack_context, int64_t *pval)
 	int64_t snum = 0;
 	int bitlen;
 	unpack_uint(unpack_context, &bitlen);
+
+	if (bitlen - 1 >= 64) {
+		unpack_context->err_no = CCPCP_RC_MALFORMED_INPUT;
+	}
+
 	if(unpack_context->err_no == CCPCP_RC_OK) {
 		const uint64_t sign_bit_mask = (uint64_t)1 << (bitlen - 1);
 		uint64_t num = unpack_context->item.as.UInt;

diff --git a/libshvchainpack/fuzz/fromChainpack.cpp b/libshvchainpack/fuzz/fromChainpack.cpp
@@ -2,6 +2,9 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     std::string err;
-    auto rpc_value = shv::chainpack::RpcValue::fromChainPack({reinterpret_cast<const char*>(data), size}, &err);
+    try {
+        auto rpc_value = shv::chainpack::RpcValue::fromChainPack({reinterpret_cast<const char*>(data), size}, &err);
+    } catch(std::exception& ex) {
+    }
     return 0;  // Values other than 0 and -1 are reserved for future use.
 }
diff --git a/libshvchainpack/src/chainpack/rpcvalue.cpp b/libshvchainpack/src/chainpack/rpcvalue.cpp
@@ -165,13 +165,27 @@ class ValueData : public RpcValue::AbstractValueData
 	RpcValue::MetaData *m_metaData = nullptr;
 };
 
+namespace {
+auto convert_to_int(const double d)
+{
+	if (std::numeric_limits<RpcValue::Int>::max() <= d) {
+		return std::numeric_limits<RpcValue::Int>::max();
+	}
+	if (std::numeric_limits<RpcValue::Int>::min() >= d) {
+		return std::numeric_limits<RpcValue::Int>::min();
+	}
+
+	return static_cast<RpcValue::Int>(d);
+}
+}
+
 class ChainPackDouble final : public ValueData<RpcValue::Type::Double, double>
 {
 	ChainPackDouble* create() override { return new ChainPackDouble(0); }
 	std::string toStdString() const override { return std::to_string(m_value); }
 	double toDouble() const override { return m_value; }
 	bool toBool() const override { return !(m_value == 0.); }
-	RpcValue::Int toInt() const override { return static_cast<RpcValue::Int>(m_value); }
+	RpcValue::Int toInt() const override { return convert_to_int(m_value); }
 	RpcValue::UInt toUInt() const override { return static_cast<RpcValue::UInt>(m_value); }
 	int64_t toInt64() const override { return static_cast<int64_t>(m_value); }
 	uint64_t toUInt64() const override { return static_cast<uint64_t>(m_value); }
@@ -189,7 +203,7 @@ class ChainPackDecimal final : public ValueData<RpcValue::Type::Decimal, RpcValu
 
 	double toDouble() const override { return m_value.toDouble(); }
 	bool toBool() const override { return !(m_value.mantisa() == 0); }
-	RpcValue::Int toInt() const override { return static_cast<RpcValue::Int>(m_value.toDouble()); }
+	RpcValue::Int toInt() const override { return convert_to_int(m_value.toDouble()); }
 	RpcValue::UInt toUInt() const override { return static_cast<RpcValue::UInt>(m_value.toDouble()); }
 	int64_t toInt64() const override { return static_cast<int64_t>(m_value.toDouble()); }
 	uint64_t toUInt64() const override { return static_cast<uint64_t>(m_value.toDouble()); }
@@ -1506,12 +1520,12 @@ void RpcValue::Decimal::setDouble(double d)
 double RpcValue::Decimal::toDouble() const
 {
 	auto ret = static_cast<double>(mantisa());
-		int exp = exponent();
-		if(exp > 0)
-			for(; exp > 0; exp--) ret *= Base;
-		else
-			for(; exp < 0; exp++) ret /= Base;
-		return ret;;
+	int exp = exponent();
+	if(exp > 0)
+		for(; exp > 0; exp--) ret *= Base;
+	else
+		for(; exp < 0; exp++) ret /= Base;
+	return ret;
 }
 
 std::string RpcValue::Decimal::toString() const

diff --git a/libshvchainpack/tests/test_chainpack.cpp b/libshvchainpack/tests/test_chainpack.cpp
@@ -484,4 +484,10 @@ DOCTEST_TEST_CASE("ChainPack")
 		REQUIRE(RpcValue::typeForName("Int bla bla", 4) == RpcValue::Type::Invalid);
 	}
 
+	DOCTEST_SUBCASE("fuzzing tests")
+	{
+		std::array<uint8_t, 15> input{0x8b, 0x8b, 0x0, 0x8d, 0xf6, 0xff, 0xff, 0xff, 0xff, 0x0, 0x8b, 0x0, 0x8e, 0x0, 0x0};
+		REQUIRE_THROWS_WITH_AS(shv::chainpack::RpcValue::fromChainPack({reinterpret_cast<const char*>(input.data()), input.size()}), "ChainPack Parse error:  error code: 4 at pos: 15 near to:\n", ParseException);
+	}
+
 }
diff --git a/libshvchainpack/tests/test_rpcvalue.cpp b/libshvchainpack/tests/test_rpcvalue.cpp
@@ -92,13 +92,10 @@ DOCTEST_TEST_CASE("RpcValue::DateTime")
 	using Parts = RpcValue::DateTime::Parts;
 	DOCTEST_SUBCASE("RpcValue::DateTime::toParts()")
 	{
-		const auto make_case = [](const string &s, const Parts &p) {
-			return tuple<string, Parts>(s, p);
-		};
 		for(const auto &[dt_str, dt_parts] : {
-			make_case("2022-01-01T00:00:00Z"s, Parts(2022, 1, 1)),
-			make_case("2023-01-23T01:02:03Z"s, Parts(2023, 1, 23, 1, 2, 3)),
-			make_case("2024-02-29T01:02:03Z"s, Parts(2024, 2, 29, 1, 2, 3)),
+			std::make_tuple("2022-01-01T00:00:00Z"s, Parts(2022, 1, 1)),
+			std::make_tuple("2023-01-23T01:02:03Z"s, Parts(2023, 1, 23, 1, 2, 3)),
+			std::make_tuple("2024-02-29T01:02:03Z"s, Parts(2024, 2, 29, 1, 2, 3)),
 		}) {
 			auto dt1 = RpcValue::DateTime::fromUtcString(dt_str);
 			auto dt2 = RpcValue::DateTime::fromParts(dt_parts);
@@ -111,3 +108,9 @@ DOCTEST_TEST_CASE("RpcValue::DateTime")
 		}
 	}
 }
+
+DOCTEST_TEST_CASE("RpcValue::Decimal")
+{
+	REQUIRE(RpcValue(RpcValue::Decimal(45, 2186)).toInt() == std::numeric_limits<RpcValue::Int>::max());
+	REQUIRE(RpcValue(RpcValue(-2.41785e+306)).toInt() == std::numeric_limits<RpcValue::Int>::min());
+}