How insecure string copy functions happen in C (cyw43.c) and how to fix it

How Insecure String Copy Functions Happen in C (cyw43.c) and How to Fix It

Summary

Three unsafe string copy calls in src/cyw43.c — including a bare strcpy() and two strncpy() calls — created buffer overflow risks in a CYW43 Wi-Fi driver emulation layer. The fix replaces all three with snprintf(), which enforces buffer size limits and guarantees null-termination in a single, consistent operation. Left unaddressed, these vulnerabilities could allow an attacker controlling input like a TAP interface name or SSID to corrupt adjacent memory and potentially execute arbitrary code.

Introduction

The src/cyw43.c file is the heart of a CYW43 Wi-Fi chip emulation layer. It manages everything from device reset state (cyw43_reset()), to TAP network bridge setup (cyw43_tap_open()), to recording Wi-Fi scan results (cyw43_add_scan_result()). These are not obscure code paths — they execute every time the emulated chip initializes, connects to a network interface, or performs a Wi-Fi scan.

Buried across three of these functions were calls to strcpy() and strncpy(): the C standard library's most notorious string-handling footguns. Semgrep's c.lang.security.insecure-use-string-copy-fn rule flagged the first occurrence at line 649, and a full review of the file revealed two more. Together, they represent a high-severity, likely exploitable attack surface in production code.

The Vulnerability Explained

Why strcpy() and strncpy() Are Dangerous

Before looking at the specific code, it's worth being precise about what each function does wrong:

• strcpy(dest, src): Copies bytes from src to dest until it encounters a null byte (\0). It performs zero bounds checking on dest. If src is longer than dest's allocated size, strcpy happily writes past the end of the buffer.

• strncpy(dest, src, n): Copies at most n bytes from src to dest. This sounds safe, but there's a critical trap: if src is n or more characters long, no null terminator is written. The destination buffer ends up without a \0, and any subsequent strlen(), printf("%s"), or further copy will read memory beyond the buffer until it stumbles upon a null byte somewhere else in the heap or stack.

The Three Vulnerable Call Sites in cyw43.c

1. cyw43_reset() — Line 649: Bare strcpy()

// BEFORE (vulnerable)
strcpy(cyw43.country, "XX");

This is the simplest case: a literal two-character string being copied into cyw43.country. At first glance it looks harmless — "XX" is short. But strcpy() is unconditionally dangerous because it expresses no intent about the buffer's capacity. If cyw43.country is ever resized, if the literal is ever replaced with a variable, or if a future developer copies this pattern with a longer string, the overflow is silent and immediate. Static analyzers correctly flag even this "safe-looking" usage because the function itself provides no safety guarantee.

2. cyw43_tap_open() — Line 675: strncpy() Without Null-Termination Guarantee

// BEFORE (vulnerable)
strncpy(cyw43.tap_name, name, sizeof(cyw43.tap_name) - 1);

The developer clearly tried to be safe here: they passed sizeof(cyw43.tap_name) - 1 as the length limit. This prevents writing beyond the buffer. However, they left out the mandatory follow-up cyw43.tap_name[sizeof(cyw43.tap_name) - 1] = '\0'. If name is exactly sizeof(cyw43.tap_name) - 1 characters or longer, the buffer contains no null terminator. The very next line:

fprintf(stderr, "[CYW43] TAP bridge enabled on '%s'\n", name);

...uses name directly, but other code that later reads cyw43.tap_name as a string will read past the buffer boundary. The name parameter comes from the caller — in a CLI tool context, this is attacker-controlled input.

3. cyw43_add_scan_result() — Line 921: strncpy() on SSID Data

// BEFORE (vulnerable)
strncpy(r->ssid, ssid, CYW43_MAX_SSID_LEN);
r->ssid[CYW43_MAX_SSID_LEN] = '\0';

This one is interesting. The developer did add a manual null-terminator on the next line. But the strncpy() call copies exactly CYW43_MAX_SSID_LEN bytes — not CYW43_MAX_SSID_LEN - 1. This means if ssid is CYW43_MAX_SSID_LEN or more characters long, strncpy fills the entire buffer without a terminator, and then the explicit r->ssid[CYW43_MAX_SSID_LEN] = '\0' writes one byte past the end of the buffer (a classic off-by-one). The ssid parameter originates from Wi-Fi scan data — in an emulated or test environment, this is fully attacker-controllable.

Attack Scenario

Consider cyw43_tap_open(). An attacker who can influence the TAP interface name passed to this function — for example, through a command-line argument or a configuration file — could supply a string longer than sizeof(cyw43.tap_name). The strncpy() call truncates the write, but without null-termination, cyw43.tap_name becomes an unterminated character array. Any subsequent code that treats it as a C string (printing, comparing, copying) will read beyond the struct boundary into adjacent heap memory. In a more complex scenario, a crafted name that precisely fills the buffer could be used to leak memory contents or, combined with other primitives, achieve controlled corruption.

The Fix

The fix replaces all three dangerous call sites with snprintf(), which is:

1. Bounded: It writes at most n bytes to the destination.
2. Always null-terminated: Unlike strncpy, snprintf always places a \0 at position n-1 if the output was truncated.
3. Universally available: Unlike strcpy_s (C11 Annex K, optional) or strlcpy (BSD/POSIX, not in C standard), snprintf is in C99 and available everywhere.

Before and After

Fix 1: cyw43_reset() — Replace strcpy with snprintf

// BEFORE
strcpy(cyw43.country, "XX");

// AFTER
snprintf(cyw43.country, sizeof(cyw43.country), "XX");

The sizeof(cyw43.country) argument tells snprintf exactly how large the destination is. Even if the literal were replaced with a longer string in the future, the write is bounded.

Fix 2: cyw43_tap_open() — Replace strncpy with snprintf

// BEFORE
strncpy(cyw43.tap_name, name, sizeof(cyw43.tap_name) - 1);

// AFTER
snprintf(cyw43.tap_name, sizeof(cyw43.tap_name), "%s", name);

Note the subtle improvement: the old code needed sizeof - 1 to leave room for a manual null terminator that was never added. snprintf handles this automatically — you pass the full buffer size, and it guarantees the output is null-terminated within that size. The "%s" format string also prevents format string injection if name ever contained % characters (though that's a secondary concern here).

Fix 3: cyw43_add_scan_result() — Replace strncpy + off-by-one with snprintf

// BEFORE
strncpy(r->ssid, ssid, CYW43_MAX_SSID_LEN);
r->ssid[CYW43_MAX_SSID_LEN] = '\0';  // off-by-one write past buffer end

// AFTER
snprintf(r->ssid, CYW43_MAX_SSID_LEN + 1, "%s", ssid);
r->ssid[CYW43_MAX_SSID_LEN] = '\0';

The snprintf call uses CYW43_MAX_SSID_LEN + 1 as the size, which is the actual allocated size of the ssid field (it holds CYW43_MAX_SSID_LEN characters plus a null terminator). The trailing explicit null assignment is now redundant but harmless — snprintf already guarantees null-termination within the specified size.

Prevention & Best Practices

Prefer snprintf Over strncpy for String Copies

The idiomatic safe pattern in portable C is:

snprintf(dest, sizeof(dest), "%s", src);

This is one line, self-documenting (the buffer size is explicit), and always produces a valid C string. There is no need for a follow-up null-termination line.

Never Use strcpy() in New Code

There is no safe use of strcpy() with runtime-variable source strings. Even with compile-time string literals (as in the cyw43_reset() case), static analyzers will — and should — flag it. Use snprintf or strlcpy instead.

Understand strncpy()'s Null-Termination Trap

If you must use strncpy(), always follow it with an explicit null-termination:

strncpy(dest, src, sizeof(dest) - 1);
dest[sizeof(dest) - 1] = '\0';

But even this pattern is error-prone (easy to forget, easy to get the index wrong). Prefer snprintf.

Use strlcpy() Where Available

On BSD systems, macOS, and with glibc 2.38+, strlcpy(dest, src, size) is the cleanest option:

strlcpy(dest, src, sizeof(dest));

It copies at most size - 1 bytes and always null-terminates. However, it is not in the C standard, so snprintf remains the most portable choice.

Enable Compiler Warnings and Static Analysis

• -Wformat-overflow and -Wstringop-overflow in GCC catch some buffer overflows at compile time.
• Semgrep with the c.lang.security.insecure-use-string-copy-fn rule flags strcpy and strncpy usage automatically.
• AddressSanitizer (-fsanitize=address) detects buffer overflows at runtime during testing.

Relevant Standards

• CWE-120: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow')
• CWE-676: Use of Potentially Dangerous Function
• CWE-787: Out-of-bounds Write
• OWASP: Buffer Overflow

Key Takeaways

• strcpy(cyw43.country, "XX") is unsafe even with a literal: Static analyzers flag strcpy unconditionally because the function itself provides no safety guarantee — the fix is snprintf regardless of how short the source string appears.
• strncpy without an explicit null terminator is a latent bug: The cyw43_tap_open() call used sizeof - 1 correctly but omitted the null-termination step, leaving cyw43.tap_name potentially unterminated when name fills the buffer.
• Off-by-one errors hide in strncpy + manual null patterns: The cyw43_add_scan_result() code wrote r->ssid[CYW43_MAX_SSID_LEN] = '\0' after copying CYW43_MAX_SSID_LEN bytes — a one-byte write past the buffer's last valid index.
• snprintf(dest, sizeof(dest), "%s", src) is the portable, one-line safe replacement: It bounds the write, guarantees null-termination, and eliminates the need for the error-prone manual null-termination pattern.
• Attacker-controlled strings flow through these functions: Both cyw43_tap_open(name) and cyw43_add_scan_result(ssid, ...) accept external input — making these buffer overflow sites directly exploitable, not merely theoretical.

How Orbis AppSec Detected This

• Source: Attacker-controlled string input via the name parameter in cyw43_tap_open() (TAP interface name from CLI/config) and ssid in cyw43_add_scan_result() (Wi-Fi scan data).
• Sink: strcpy(cyw43.country, "XX") at src/cyw43.c:649; strncpy(cyw43.tap_name, name, sizeof(cyw43.tap_name) - 1) at src/cyw43.c:675; strncpy(r->ssid, ssid, CYW43_MAX_SSID_LEN) at src/cyw43.c:921.
• Missing control: No enforcement of destination buffer size (in the strcpy case) and no guaranteed null-termination (in both strncpy cases).
• CWE: CWE-120 (Buffer Copy without Checking Size of Input) and CWE-676 (Use of Potentially Dangerous Function).
• Fix: All three call sites replaced with snprintf(dest, sizeof(dest), "%s", src), which enforces the buffer limit and always null-terminates the result.

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

The three insecure string copy calls in src/cyw43.c illustrate a pattern that's endemic in C codebases: developers reaching for strncpy as a "safe" alternative to strcpy, only to trip over its non-obvious null-termination behavior. The off-by-one in cyw43_add_scan_result() is particularly instructive — the developer clearly tried to be careful, added a manual null-terminator, and still got it wrong because strncpy's semantics make it easy to miscalculate the index.

The fix is simple and consistent: snprintf(dest, sizeof(dest), "%s", src) handles bounds checking and null-termination in a single, readable call. Adopting this pattern uniformly across a codebase — and enforcing it with static analysis rules — eliminates an entire class of memory safety bugs before they reach production.

References

• CWE-120: Buffer Copy without Checking Size of Input
• CWE-676: Use of Potentially Dangerous Function
• CWE-787: Out-of-bounds Write
• OWASP Buffer Overflow Vulnerability
• OWASP C-Based Toolchain Hardening Cheat Sheet
• cppreference: snprintf
• Semgrep rule: insecure-use-string-copy-fn
• fix: use bounded strlcpy/snprintf in cyw43.c...