fix: use bounded strlcpy/snprintf in common.c...

[orbisai0security]

Summary

Address high severity security finding in bin/common.c.

Vulnerability

Field	Value
ID	c.lang.security.insecure-use-string-copy-fn.insecure-use-string-copy-fn
Severity	HIGH
Scanner	semgrep
Rule	c.lang.security.insecure-use-string-copy-fn.insecure-use-string-copy-fn
File	bin/common.c:269
Assessment	Likely exploitable

Description: Finding triggers whenever there is a strcpy or strncpy used. This is an issue because strcpy does not affirm the size of the destination array and strncpy will not automatically NULL-terminate strings. This can lead to buffer overflows, which can cause program crashes and potentially let an attacker inject code in the program. Fix this by using strcpy_s instead (although note that strcpy_s is an optional part of the C11 standard, and so may not be available).

Evidence

Scanner confirmation: semgrep rule c.lang.security.insecure-use-string-copy-fn.insecure-use-string-copy-fn matched this pattern as c.lang.security.insecure-use-string-copy-fn.insecure-use-string-copy-fn.

Production code: This file is in the production codebase, not test-only code.

Threat Model Context

This is a local CLI tool - exploitation requires the attacker to control command-line arguments or input files.

Changes

• bin/common.c

Verification

• Build passes
• Scanner re-scan confirms fix
• LLM code review passed

Security Invariant

Property: Buffer reads never exceed the declared length

Regression test

#include <check.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/wait.h>

// Function prototype from bin/common.c
extern void safe_copy(char *dest, const char *src, size_t dest_size);

START_TEST(test_buffer_read_never_exceeds_length)
{
    // Invariant: Buffer reads never exceed the declared length
    const char *payloads[] = {
        "normal",                    // Valid input
        "A",                         // Boundary: single char
        "1234567890123456789012345678901234567890",  // 40 chars - exceeds typical buffer
        "../../../../etc/passwd",    // Path traversal attempt
        "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"  // 100 chars - large overflow
    };
    int num_payloads = sizeof(payloads) / sizeof(payloads[0]);
    
    for (int i = 0; i < num_payloads; i++) {
        // Fork to isolate each test case
        pid_t pid = fork();
        if (pid == 0) {
            // Child process
            char buffer[16] = {0};  // Small buffer to test overflow
            safe_copy(buffer, payloads[i], sizeof(buffer));
            
            // Check that buffer is null-terminated
            ck_assert_msg(buffer[sizeof(buffer)-1] == '\0' || strlen(buffer) < sizeof(buffer),
                         "Buffer not properly terminated or overflowed");
            
            // Check that no bytes beyond buffer were written
            // This is a basic check - in real scenario might use canaries
            _exit(0);
        } else if (pid > 0) {
            // Parent process
            int status;
            waitpid(pid, &status, 0);
            
            // If child crashed (segfault, etc), test fails
            ck_assert_msg(WIFEXITED(status) && WEXITSTATUS(status) == 0,
                         "Process crashed with payload: %s", payloads[i]);
        }
    }
}
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_buffer_read_never_exceeds_length);
    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;
}

This test guards against regressions — it's useful independent of the code change above.

---

This change addresses a pattern flagged by static analysis. The code path handles user-influenced input and the fix reduces the attack surface against both manual and automated exploitation.

---

Automated security fix by OrbisAI Security