Merge pull request #264 from chaoticgd/bounds_checks

Fix some broken bounds checks

CMakeLists.txt

@@ -102,6 +102,7 @@ set(TEST_SOURCES
 	test/ccc/mdebug_importer_tests.cpp
 	test/ccc/stabs_tests.cpp
 	test/ccc/symbol_database_tests.cpp
+	test/ccc/util_tests.cpp
 )
 
 add_executable(demangle src/demangle.cpp)

src/ccc/util.cpp

@@ -53,8 +53,8 @@ void set_custom_error_callback(CustomErrorCallback callback)
 
 const char* get_string(std::span<const u8> bytes, u64 offset)
 {
-	for (const unsigned char* c = bytes.data() + offset; c < bytes.data() + bytes.size(); c++) {
-		if (*c == '\0') {
+	for (u64 i = offset; i < bytes.size(); i++) {
+		if (bytes[i] == '\0') {
 			return (const char*) &bytes[offset];
 		}
 	}

src/ccc/util.h

@@ -210,7 +210,7 @@ void warn_impl(const char* source_file, int source_line, const char* format, Arg
 template <typename T>
 const T* get_aligned(std::span<const u8> bytes, u64 offset)
 {
-	if (offset + sizeof(T) > bytes.size() || offset % alignof(T) != 0) {
+	if (offset + sizeof(T) > bytes.size() || offset + sizeof(T) < offset || offset % alignof(T) != 0) {
 		return nullptr;
 	}
 
@@ -220,7 +220,7 @@ const T* get_aligned(std::span<const u8> bytes, u64 offset)
 template <typename T>
 const T* get_unaligned(std::span<const u8> bytes, u64 offset)
 {
-	if (offset + sizeof(T) > bytes.size()) {
+	if (offset + sizeof(T) > bytes.size() || offset + sizeof(T) < offset) {
 		return nullptr;
 	}
 
@@ -230,7 +230,7 @@ const T* get_unaligned(std::span<const u8> bytes, u64 offset)
 template <typename T>
 const std::optional<T> copy_unaligned(std::span<const u8> bytes, u64 offset)
 {
-	if (offset + sizeof(T) > bytes.size()) {
+	if (offset + sizeof(T) > bytes.size() || offset + sizeof(T) < offset) {
 		return std::nullopt;
 	}
 

test/ccc/util_tests.cpp

@@ -0,0 +1,53 @@
+// This file is part of the Chaos Compiler Collection.
+// SPDX-License-Identifier: MIT
+
+#include <gtest/gtest.h>
+#include "ccc/util.h"
+
+using namespace ccc;
+
+#define DEREF_OR_ZERO(x) ((x) ? (*(x)) : 0)
+
+TEST(CCCUtil, GetAligned)
+{
+	u8 data[7] = {1, 0, 0, 1, 0, 0, 1};
+
+	EXPECT_EQ(DEREF_OR_ZERO(get_aligned<u32>(data, 0)), 0x01000001);
+	EXPECT_EQ(get_aligned<u32>(data, 1), nullptr);
+	EXPECT_EQ(get_aligned<u32>(data, 4), nullptr);
+	EXPECT_EQ(get_aligned<u32>(data, 7), nullptr);
+	EXPECT_EQ(get_aligned<u32>(data, 8), nullptr);
+	EXPECT_EQ(get_aligned<u32>(data, 0xfffffffffffffffc), nullptr);
+}
+
+TEST(CCCUtil, GetUnaligned)
+{
+	u8 data[7] = {1, 2, 3, 4, 5, 6, 7};
+
+	EXPECT_EQ(DEREF_OR_ZERO(get_unaligned<u8>(data, 0)), 1);
+	EXPECT_EQ(DEREF_OR_ZERO(get_unaligned<u8>(data, 1)), 2);
+	EXPECT_EQ(get_unaligned<u8>(data, 8), nullptr);
+	EXPECT_EQ(get_unaligned<u8>(data, 0xffffffffffffffff), nullptr);
+}
+
+TEST(CCCUtil, CopyUnaligned)
+{
+	u8 data[7] = {1, 0, 0, 1, 0, 0, 1};
+
+	EXPECT_EQ(DEREF_OR_ZERO(copy_unaligned<u32>(data, 0)), 0x01000001);
+	EXPECT_EQ(DEREF_OR_ZERO(copy_unaligned<u32>(data, 3)), 0x01000001);
+	EXPECT_EQ(copy_unaligned<u32>(data, 4).has_value(), false);
+	EXPECT_EQ(copy_unaligned<u32>(data, 8).has_value(), false);
+	EXPECT_EQ(copy_unaligned<u32>(data, 0xffffffffffffffff).has_value(), false);
+}
+
+TEST(CCCUtil, GetString)
+{
+	u8 data[7] = {'h', 'e', 'l', 'l', 'o', '\0', '!'};
+
+	EXPECT_EQ(get_string(data, 0), std::string("hello"));
+	EXPECT_EQ(get_string(data, 5), std::string(""));
+	EXPECT_EQ(get_string(data, 6), nullptr);
+	EXPECT_EQ(get_string(data, 7), nullptr);
+	EXPECT_EQ(get_string(data, 0xffffffffffffffff), nullptr);
+}