How insecure string copy functions happen in C apputils.c and how to fix it

How Insecure String Copy Functions Happen in C apputils.c and How to Fix It

Introduction

The file src/apps/common/apputils.c is a foundational utility module — it handles low-level operations like binding sockets to network interfaces (sock_bind_to_device()) and resolving domain names (getdomainname()). These are exactly the kinds of functions that process external, attacker-influenced input: network interface names supplied on the command line and domain name strings returned from OS APIs. A flaw in how strings were copied into fixed-size buffers inside these functions created a high-severity buffer overflow risk that could corrupt memory or crash the process.

Semgrep's c.lang.security.insecure-use-string-copy-fn rule flagged four uses of strncpy() in this file. The root cause is subtle but dangerous: strncpy() will not null-terminate the destination buffer when the source string is exactly as long as or longer than the specified copy size. Code that subsequently reads or passes that buffer as a C string will walk past the intended boundary, with unpredictable results.

The Vulnerability Explained

What strncpy() actually does (and doesn't do)

Most C developers reach for strncpy() as the "safe" alternative to strcpy(). The reasoning is intuitive: you pass a maximum length, so it can't overflow — right? Unfortunately, that's only half the story.

The C standard specifies that if the source string is at least as long as n, strncpy() copies exactly n characters and does not append a null terminator. The destination buffer is left without a '\0' at position n, meaning any subsequent strlen(), printf("%s", ...), or further copy operation will read beyond the intended end of the string.

The vulnerable code — sock_bind_to_device() at line 249

// BEFORE (vulnerable)
struct ifreq ifr;
memset(&ifr, 0, sizeof(ifr));

strncpy(ifr.ifr_name, (const char *)ifname, sizeof(ifr.ifr_name));

if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, (const void *)&ifr, sizeof(ifr)) < 0) {

ifr.ifr_name is a fixed-size character array (16 bytes on Linux, defined as IFNAMSIZ). If ifname is exactly 16 characters long, strncpy() fills all 16 bytes with source characters and writes no null terminator. The kernel's SO_BINDTODEVICE handler then reads ifr.ifr_name as a C string, potentially running off the end of the field into adjacent struct members.

The vulnerable code — getdomainname() at lines 952, 972, and 992

The same pattern appears three times inside getdomainname(), once for each branch (DomainForestName, DomainNameDns, DomainNameFlat):

// BEFORE (vulnerable) — repeated in three branches
if (nOutSize > len - 1) {
  n = len - 1;
}
strncpy(name, pszOut, n);
name[n] = 0;   // Manual null-termination attempt

At first glance, the name[n] = 0 line looks like it fixes the null-termination problem. But look carefully: n is bounded to len - 1, and strncpy(name, pszOut, n) copies at most n characters. The manual name[n] = 0 sets the byte at index n to zero — which is correct only if strncpy copied exactly n characters. If pszOut is shorter than n, strncpy pads with nulls (harmless but wasteful). If pszOut is longer than or equal to n, strncpy copies n bytes without a terminator, and then name[n] = 0 correctly terminates — but this is still fragile, error-prone manual bookkeeping that a safer API eliminates entirely.

Attack scenario

This is a local CLI tool, and ifname comes from command-line arguments. An attacker (or a malicious config file) supplying a 16-character interface name like "eth0eth0eth0eth0" to sock_bind_to_device() would cause strncpy to fill ifr.ifr_name with 16 bytes and no terminator. Depending on compiler layout and padding, the kernel socket call could read garbage bytes from adjacent struct memory, potentially causing a bind failure, a crash, or — in a more complex exploit chain — leaking stack data or influencing control flow.

The Fix

The fix replaces all four strncpy() calls with snprintf(dest, size, "%s", src). This is a well-established idiom that:

1. Always null-terminates — snprintf guarantees a '\0' at position size - 1 regardless of source length.
2. Respects the buffer boundary — it will never write more than size bytes including the terminator.
3. Eliminates manual bookkeeping — no need for the name[n] = 0 line.

Fix 1 — sock_bind_to_device() (line 249)

// BEFORE
strncpy(ifr.ifr_name, (const char *)ifname, sizeof(ifr.ifr_name));

// AFTER
snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", (const char *)ifname);

sizeof(ifr.ifr_name) is passed directly as the size argument, so snprintf will copy at most sizeof(ifr.ifr_name) - 1 characters and always write a null terminator at the last byte. The kernel receives a properly terminated interface name string.

Fix 2 — getdomainname() (lines 952, 972, 992 — three branches)

// BEFORE (all three branches)
strncpy(name, pszOut, n);
name[n] = 0;

// AFTER (all three branches)
snprintf(name, n + 1, "%s", pszOut);

The size argument n + 1 is deliberate and correct: n was already clamped to len - 1 by the preceding bounds check, so n + 1 <= len. Passing n + 1 to snprintf tells it: "write at most n characters of content, then a null terminator." This exactly replicates the intended behavior of the old strncpy + manual null-termination pattern, but does so safely and atomically. The redundant name[n] = 0 line is removed because snprintf handles it.

Before/After Summary

Prevention & Best Practices

Drop strncpy() from your C coding standards

strncpy() was designed for a specific historical use case (fixed-width, not null-terminated record fields in early Unix filesystems) and is routinely misused as a "safe strcpy." Consider it deprecated for general string handling.

Use snprintf() for portable, safe string copies

// Safe, portable, always null-terminates
snprintf(dest, sizeof(dest), "%s", src);

This pattern works everywhere, requires no extra headers, and is immediately obvious to any C reader.

Enable compiler and sanitizer warnings

• Compile with -Wall -Wextra — GCC/Clang will warn on some dangerous string patterns.
• Use AddressSanitizer (-fsanitize=address) during development and testing to catch buffer overflows at runtime.
• Use UndefinedBehaviorSanitizer (-fsanitize=undefined) to catch related undefined behavior.

Add static analysis to your CI pipeline

Semgrep's c.lang.security.insecure-use-string-copy-fn rule (used here) is free and fast. Add it to your CI pipeline to catch strcpy/strncpy regressions before they reach production:

# .github/workflows/semgrep.yml
- uses: semgrep/semgrep-action@v1
  with:
    config: "p/c"

Relevant standards

• CWE-120: Buffer Copy without Checking Size of Input ("Classic Buffer Overflow")
• CWE-676: Use of Potentially Dangerous Function
• OWASP: Buffer Overflow
• SEI CERT C Coding Standard: STR07-C — Use the bounds-checking interfaces for string manipulation

Key Takeaways

• strncpy() is not a safe drop-in for strcpy() — in sock_bind_to_device(), passing an interface name exactly 16 characters long silently omits the null terminator, corrupting the ifreq struct passed to the kernel.
• Manual null-termination (name[n] = 0) is fragile — the three getdomainname() branches each tried to compensate with an explicit null write, but this pattern is easy to get wrong and easy to accidentally delete during refactoring.
• snprintf(dest, size, "%s", src) is the portable, idiomatic replacement — it handles both the length bound and null-termination atomically, reducing the code from two lines to one.
• Network interface names and domain name strings are attacker-influenced — even in a local CLI tool, these values come from command-line arguments or OS APIs and should always be treated as untrusted input.
• Static analysis caught what code review missed — four separate call sites carried this pattern; automated scanning with Semgrep found all of them in a single pass.

How Orbis AppSec Detected This

• Source: The ifname parameter in sock_bind_to_device() originates from command-line arguments (attacker-controlled); pszOut in getdomainname() comes from a Windows API call that converts wide-character domain name strings.
• Sink: strncpy(ifr.ifr_name, (const char *)ifname, sizeof(ifr.ifr_name)) at src/apps/common/apputils.c:249, and strncpy(name, pszOut, n) at lines 952, 972, and 992.
• Missing control: No guarantee of null-termination when the source string length equals or exceeds the destination buffer size; the manual name[n] = 0 pattern in getdomainname() partially compensated but remained fragile and inconsistent.
• CWE: CWE-120 — Buffer Copy without Checking Size of Input; CWE-676 — Use of Potentially Dangerous Function.
• Fix: All four strncpy() calls were replaced with snprintf(dest, size, "%s", src), which enforces both the size bound and null-termination in a single, portable call.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

Buffer overflows caused by strncpy() misuse are among the oldest and most well-documented vulnerabilities in C programming — yet they continue to appear in production codebases because strncpy() looks safe. The four call sites in apputils.c are a textbook example: the function takes a size argument, so developers assume it's bounded. But the missing null-terminator guarantee means the apparent safety is illusory.

The fix is straightforward: snprintf(dest, size, "%s", src) is portable, always null-terminates, and requires no additional manual bookkeeping. Adopting this pattern consistently across a C codebase — and enforcing it with a static analysis rule in CI — eliminates an entire class of memory safety bugs before they can be exploited.

References

• CWE-120: Buffer Copy without Checking Size of Input
• CWE-676: Use of Potentially Dangerous Function
• OWASP Buffer Overflow Vulnerability
• OWASP C-Based Toolchain Hardening Cheat Sheet
• SEI CERT C Coding Standard — STR07-C
• Semgrep rule: c.lang.security.insecure-use-string-copy-fn
• cppreference — snprintf
• fix: use bounded strlcpy/snprintf in apputils.c...