fix: use bounded strlcpy/snprintf in vm.c

src/vm.c

@@ -403,59 +403,57 @@ static bool serialize_printer(NoctEnv *env, char *buf, size_t size, NoctValue *v
 	case NOCT_VALUE_INT:
 		if (!noct_get_int(env, value, (int32_t *)&ival))
 			return false;
-		snprintf(digits, sizeof(digits), "%d", ival);
-		strncat(buf, digits, size);
+		snprintf(buf + strlen(buf), size - strlen(buf), "%d", ival);
 		break;
 	case NOCT_VALUE_FLOAT:
 		if (!noct_get_float(env, value, &fval))
 			return false;
-		snprintf(digits, sizeof(digits), "%f", fval);
-		strncat(buf, digits, size);
+		snprintf(buf + strlen(buf), size - strlen(buf), "%f", fval);
 		break;
 	case NOCT_VALUE_STRING:
 		if (!noct_get_string(env, value, &sval))
 			return false;
 		if (is_inside_obj)
-			strncat(buf, "\"", size);
-		strncat(buf, sval, size);
+			snprintf(buf + strlen(buf), size - strlen(buf), "\"");
+		snprintf(buf + strlen(buf), size - strlen(buf), "%s", sval);
 		if (is_inside_obj)
-			strncat(buf, "\"", size);
+			snprintf(buf + strlen(buf), size - strlen(buf), "\"");
 		break;
 	case NOCT_VALUE_ARRAY:
 		if (!noct_get_array_size(env, value, &items))
 			return false;
-		strncat(buf, "[", size);
+		snprintf(buf + strlen(buf), size - strlen(buf), "[");
 		for (i = 0; i < items; i++) {
 			NoctValue elem;
 			if (!noct_get_array_elem(env, value, i, &elem))
 				return false;
 			if (!serialize_printer(env, buf, size, &elem, true))
 				return false;
 			if (i != items - 1)
-				strncat(buf, ", ", size);
+				snprintf(buf + strlen(buf), size - strlen(buf), ", ");
 		}
-		strncat(buf, "]", size);
+		snprintf(buf + strlen(buf), size - strlen(buf), "]");
 		break;
 	case NOCT_VALUE_DICT:
 		if (!noct_get_dict_size(env, value, &items))
 			return false;
-		strncat(buf, "{", size);
+		snprintf(buf + strlen(buf), size - strlen(buf), "{");
 		for (i = 0; i < items; i++) {
 			NoctValue k, v;
 			if (!noct_get_dict_by_index(env, value, i, &k, &v))
 				return false;
 			if (!noct_get_string(env, &k, &sval))
 				return false;
-			strncat(buf, sval, size);
-			strncat(buf, ": ", size);
+			snprintf(buf + strlen(buf), size - strlen(buf), "%s", sval);
+			snprintf(buf + strlen(buf), size - strlen(buf), ": ");
 			serialize_printer(env, buf, size, &v, true);
 			if (i != items - 1)
-				strncat(buf, ", ", size);
+				snprintf(buf + strlen(buf), size - strlen(buf), ", ");
 		}
-		strncat(buf, "}", size);
+		snprintf(buf + strlen(buf), size - strlen(buf), "}");
 		break;
 	case NOCT_VALUE_FUNC:
-		strncat(buf, "<func>", size);
+		snprintf(buf + strlen(buf), size - strlen(buf), "<func>");
 		break;
 	default:
 		assert(0);
@@ -1441,7 +1439,8 @@ deserialize_save_data_recursively(
 			char key[SER_STRING_MAX];
 			if (!ser_get_string(ctx, &sval))
 				return false;
-			strcpy(key, sval);
+			strncpy(key, sval, sizeof(key) - 1);
+			key[sizeof(key) - 1] = '\0';
 			if (!deserialize_save_data_recursively(env, &elem, ctx))
 				return false;
 			if (!noct_set_dict_elem_cstr(env, value, key, &elem))

tests/test_invariant_vm.c

@@ -0,0 +1,82 @@
+#include <check.h>
+#include <stdlib.h>
+#include <string.h>
+#include <setjmp.h>
+#include <signal.h>
+
+static jmp_buf jump_buffer;
+static volatile sig_atomic_t segfault_occurred = 0;
+
+static void segfault_handler(int sig) {
+    segfault_occurred = 1;
+    longjmp(jump_buffer, 1);
+}
+
+START_TEST(test_vm_string_copy_bounds)
+{
+    // Invariant: VM string operations must not corrupt stack or cause crashes regardless of input size
+    const char *payloads[] = {
+        "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",  // 256 bytes - exploit case
+        "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",  // 128 bytes - boundary
+        "valid_key"  // valid input
+    };
+    int num_payloads = sizeof(payloads) / sizeof(payloads[0]);
+
+    for (int i = 0; i < num_payloads; i++) {
+        segfault_occurred = 0;
+        signal(SIGSEGV, segfault_handler);
+        signal(SIGABRT, segfault_handler);
+
+        if (setjmp(jump_buffer) == 0) {
+            char script[4096];
+            snprintf(script, sizeof(script), "d = {%s: 1}; return d;", payloads[i]);
+
+            FILE *f = fopen("/tmp/test_vm_script.txt", "w");
+            ck_assert_ptr_nonnull(f);
+            fprintf(f, "%s", script);
+            fclose(f);
+
+            int ret = system("timeout 2 ./vm_test_harness /tmp/test_vm_script.txt 2>/dev/null");
+            ck_assert_msg(WIFEXITED(ret) && (WEXITSTATUS(ret) == 0 || WEXITSTATUS(ret) == 1),
+                         "VM must handle oversized strings without crashing (payload %d)", i);
+        } else {
+            ck_abort_msg("Stack corruption or segfault detected with payload %d", i);
+        }
+
+        signal(SIGSEGV, SIG_DFL);
+        signal(SIGABRT, SIG_DFL);
+    }
+
+    unlink("/tmp/test_vm_script.txt");
+}
+END_TEST
+
+Suite *security_suite(void)
+{
+    Suite *s;
+    TCase *tc_core;
+
+    s = suite_create("Security");
+    tc_core = tcase_create("Core");
+
+    tcase_add_test(tc_core, test_vm_string_copy_bounds);
+    suite_add_tcase(s, tc_core);
+
+    return s;
+}
+
+int main(void)
+{
+    int number_failed;
+    Suite *s;
+    SRunner *sr;
+
+    s = security_suite();
+    sr = srunner_create(s);
+
+    srunner_run_all(sr, CK_NORMAL);
+    number_failed = srunner_ntests_failed(sr);
+    srunner_free(sr);
+
+    return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}