How integer overflow in buffer size calculation happens in C and how to fix it

How Integer Overflow in Buffer Size Calculation Happens in C and How to Fix It

Introduction

In the NSH (NuttX Shell) project, a critical vulnerability was discovered in nshlib/nsh_vars.c at line 249. The nsh_setvar() function—responsible for storing shell variables—contained an unchecked integer addition that could wrap around, causing a heap buffer overflow. An attacker with shell access could exploit this by setting a variable with a carefully crafted name and value, triggering memory corruption.

The vulnerable code pattern was deceptively simple:

newsize = pstate->varsz + varlen;
newvarp = (FAR char *)realloc(pstate->varp, newsize);

Here, varlen is calculated as strlen(name) + strlen(value) + 2, and pstate->varsz holds the current buffer size. If an attacker controls the variable name and value, they can make this addition overflow, wrapping the result to a small number. realloc() then allocates a tiny buffer, and the subsequent snprintf() writes far beyond its bounds.

This vulnerability matters because NSH is a production CLI tool, and any local user with shell access becomes a potential attacker. The fix was straightforward but critical: validate that the addition won't overflow before performing it.

The Vulnerability Explained

What Went Wrong

Integer overflow in buffer size calculations is a classic C vulnerability, yet it remains common because the fix is often overlooked in code reviews.

The vulnerable code (before fix):

// nshlib/nsh_vars.c:249-260 (simplified)
int nsh_setvar(FAR struct nsh_vtbl_s *vtbl, FAR const char *name,
               FAR const char *value)
{
  struct nsh_pstate_s *pstate = (struct nsh_pstate_s *)vtbl;
  size_t varlen = strlen(name) + strlen(value) + 2;

  if (pstate->varp != NULL)
    {
      // VULNERABLE: No overflow check!
      newsize = pstate->varsz + varlen;  // Line 257 in original
      newvarp = (FAR char *)realloc(pstate->varp, newsize);
      if (newvarp == NULL)
        {
          return -ENOMEM;
        }
      // ... snprintf writes to newvarp
    }
}

Why this is exploitable:

Let's say pstate->varsz is currently 1000 bytes. An attacker executes:

set AAAA...AAAA=BBBB...BBBB

Where the combined length of name + value + 2 is crafted such that:
- varlen = 0xFFFFFF00 (a very large number)
- newsize = 1000 + 0xFFFFFF00 = 0x100000300

If newsize is stored in a 32-bit int, this wraps to 0x300 (768 bytes). The realloc() allocates only 768 bytes, but the code then writes the full variable data (name + value + separators) into this undersized buffer, corrupting the heap.

Attack scenario:

1. Attacker gains local NSH shell access (e.g., via SSH, serial console, or local execution)
2. Attacker crafts a set command with a variable name and value designed to overflow the size calculation
3. The overflow causes realloc() to allocate a small buffer
4. Subsequent writes overflow the heap, corrupting adjacent memory structures
5. Depending on heap layout, this could lead to information disclosure or code execution

Real-world impact:

For a local CLI tool, this is a privilege escalation risk. If NSH runs with elevated privileges (e.g., as part of a system utility), an unprivileged user could corrupt memory and potentially execute arbitrary code in that privileged context.

The Fix

What Changed

The fix adds a single overflow check before the vulnerable addition. Here's the exact diff:

  if (pstate->varp != NULL)
    {
+     if (varlen > INT_MAX - pstate->varsz)
+       {
+         return -ENOMEM;
+       }
+
      newsize = pstate->varsz + varlen;
      newvarp = (FAR char *)realloc(pstate->varp, newsize);
      if (newvarp == NULL)
        {
          return -ENOMEM;
        }

How it works:

The check if (varlen > INT_MAX - pstate->varsz) detects overflow before it happens:

• INT_MAX - pstate->varsz gives the maximum value that varlen can safely be
• If varlen exceeds this, the addition would overflow
• Instead of allowing the overflow, the function returns -ENOMEM (memory allocation failure)

Why this is correct:

1. Safe arithmetic: The check itself uses only subtraction, which cannot overflow (since pstate->varsz is always ≤ INT_MAX)
2. Fail-safe: Returns an error code, which the caller must handle—it doesn't silently corrupt memory
3. Minimal performance impact: One comparison per variable assignment

Security Improvement

Before the fix:
- Attacker can trigger heap overflow with crafted input
- Memory corruption can lead to information disclosure or code execution
- No bounds on the variable size that would cause the overflow

After the fix:
- Overflow attempts are caught and rejected
- The function fails safely with -ENOMEM
- The security boundary is maintained: "Buffer size calculation must not overflow and must allocate sufficient memory"

Prevention & Best Practices

For C Developers

1. Always validate arithmetic before security-critical operations:

// BAD: No overflow check
size_t total = size_a + size_b;
char *buffer = malloc(total);

// GOOD: Check before adding
if (size_a > SIZE_MAX - size_b) {
    return -ENOMEM;  // Overflow would occur
}
size_t total = size_a + size_b;
char *buffer = malloc(total);

2. Use safe integer libraries when available:

• CERT Secure Coding: Use checked_add() from safe-int.h (if available in your environment)
• OpenBSD: sys/sys/overflow.h provides __builtin_add_overflow() (GCC/Clang)
• Rust FFI: If wrapping C code, consider using Rust's overflow-checking integers

3. Prefer size_t for memory sizes:

The original code used int for newsize, which is smaller than size_t on 64-bit systems. Using size_t for all buffer sizes reduces overflow risk:

// Safer approach
size_t varlen = strlen(name) + strlen(value) + 2;
size_t newsize = pstate->varsz + varlen;  // size_t, not int

4. Add regression tests:

The PR includes a comprehensive regression test that validates the security invariant:

// Verify: allocated size >= expected size (no overflow)
ck_assert_msg(allocated_size >= expected_size,
             "Buffer overflow: allocated %zu, needed %zu",
             allocated_size, expected_size);

Detection & Tooling

Static Analysis:

• Clang Static Analyzer: Detects unchecked arithmetic leading to buffer operations
• Coverity: Flags integer overflow in security-sensitive paths
• Semgrep: Custom rules can detect patterns like realloc(ptr, a + b) without overflow checks

Runtime Protection:

• AddressSanitizer (ASan): Catches heap buffer overflows at runtime
• UBSan: Detects undefined behavior from integer overflow (with -fsanitize=signed-integer-overflow)

Standards & References

• CWE-190: Integer Overflow or Wraparound
• CWE-122: Heap-based Buffer Overflow
• CERT Secure Coding: INT32-C: Ensure that operations on signed integers do not result in overflow

Key Takeaways

• Integer overflow in buffer calculations is subtle but critical: A single unchecked addition in nsh_setvar() could corrupt heap memory and enable code execution.

• The fix is simple but must be applied consistently: The PR notes that line 293 in the same file may have a similar pattern and needs review—suggesting this vulnerability class is easy to miss.

• Overflow checks must happen before the arithmetic: Checking if (varlen > INT_MAX - pstate->varsz) prevents the overflow from ever occurring, rather than trying to detect it after the fact.

• Local attackers can exploit this: NSH is a CLI tool, but local code execution via heap corruption is still a serious privilege escalation risk.

• Regression tests are essential: The included test explicitly validates that buffer allocation sizes never overflow, protecting against future reintroduction of this bug.

How Orbis AppSec Detected This

Source: Variable name and value provided by the user via the NSH set command (e.g., set VARNAME=value)

Sink: The unchecked arithmetic newsize = pstate->varsz + varlen at line 257 in nshlib/nsh_vars.c, followed by realloc(pstate->varp, newsize)

Missing control: No validation that the addition would not overflow the int type before performing it

CWE: CWE-190 (Integer Overflow or Wraparound) leading to CWE-122 (Heap-based Buffer Overflow)

Fix: Added overflow check: if (varlen > INT_MAX - pstate->varsz) return -ENOMEM; before the vulnerable addition

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

Integer overflow in buffer size calculations is a reminder that even simple arithmetic operations require careful validation in security-critical code. The fix applied to nsh_setvar() is minimal—just five lines—but it closes a critical attack vector that could have allowed local privilege escalation or information disclosure.

This vulnerability exemplifies why automated security scanning and code review are essential. The pattern is easy to overlook in manual review, yet simple static analysis tools can catch it reliably. By combining automated detection with regression testing, we can prevent this class of vulnerability from recurring.

If you're maintaining C code that handles user-controlled sizes, variable-length data, or memory allocation, audit your arithmetic operations now. Check for unchecked additions, multiplications, and other operations before they're passed to malloc(), realloc(), or buffer operations. The few extra lines of validation code can prevent critical security breaches.

References

• CWE-190: Integer Overflow or Wraparound
• CWE-122: Heap-based Buffer Overflow
• CERT Secure Coding: INT32-C
• OWASP: Buffer Overflow
• Semgrep: Integer Overflow Detection
• fix: in nsh_setvar(), the calculation `newsize = pst... in nsh_vars.c