How integer overflow in CsoundMYFLTArray constructor happens in C++ and how to fix it

How Integer Overflow in CsoundMYFLTArray Constructor Happens in C++ and How to Fix It

Introduction

The Java/cs_glue.cpp file is the SWIG-generated glue layer that bridges Csound's C++ engine to Java (and by extension, Android and other JVM consumers). It handles the lifecycle of audio buffers, argument vectors, and other low-level objects that cross the native/managed boundary. A flaw in the CsoundMYFLTArray constructor — specifically on line 324 — meant that any caller passing a sufficiently large n through the Java/SWIG binding could silently corrupt the heap.

The root cause is a single unchecked multiplication: (size_t) n * sizeof(MYFLT). That expression looks safe because of the cast, but the cast only changes the type of the overflow — it does not prevent it.

The Vulnerability Explained

What the code was doing

// VULNERABLE — cs_glue.cpp line 322–328 (before fix)
CsoundMYFLTArray::CsoundMYFLTArray(int n)
{
    p = (MYFLT*) 0;
    pp = (void*) 0;
    if (n > 0)
      pp = (void*) malloc((size_t) n * sizeof(MYFLT));  // ← line 324
    if (pp) {
      p = (MYFLT*) pp;
      for (int i = 0; i < n; i++)
        ...

The intent is clear: allocate n elements of type MYFLT (a double on most platforms, so sizeof(MYFLT) == 8). The only guard in place is n > 0, which is necessary but nowhere near sufficient.

Why the cast doesn't help

When n is, say, INT_MAX (2,147,483,647 on a 32-bit int):

(size_t) n * sizeof(MYFLT)
= (size_t) 2147483647 * 8
= 17179869176

On a 64-bit platform that value fits in size_t, so malloc is asked for ~16 GiB and will almost certainly return NULL. That case is caught by the if (pp) check.

But the dangerous boundary is one step lower. Consider n = SIZE_MAX / sizeof(MYFLT) + 1:

(size_t) n * sizeof(MYFLT)
= (SIZE_MAX / 8 + 1) * 8
= SIZE_MAX + 8           ← wraps to 7 on a 64-bit platform

Now malloc(7) succeeds and returns a valid 7-byte pointer. The if (pp) guard passes. The subsequent initialization loop then writes n doubles (billions of bytes) starting at that 7-byte allocation, smashing everything on the heap.

The second overflow: CsoundArgVList::Insert

The same PR also fixed a parallel issue in CsoundArgVList::Insert:

// VULNERABLE — cs_glue.cpp line 404–405 (before fix)
new_cnt = (cnt >= 0 ? cnt + 1 : 1);
new_argv = (char**) malloc((size_t) (new_cnt + 1) * sizeof(char*));

Here cnt + 1 can overflow a signed int when cnt == INT_MAX, and (new_cnt + 1) * sizeof(char*) can overflow the size_t multiplication, again producing an undersized allocation.

Exploitation scenario

An attacker who controls the argument list fed to the Csound engine through the Java binding — for example, via a crafted .csd file or a malicious plugin — can call CsoundMYFLTArray(n) with n = SIZE_MAX / sizeof(MYFLT) + 1. The constructor allocates 7 bytes, then the loop writes gigabytes of zeroes, overwriting adjacent heap metadata, function pointers, and data. On modern systems this typically results in a crash, but on hardened targets it can be leveraged for arbitrary code execution.

The Fix

Change 1 — CsoundMYFLTArray constructor

// FIXED — cs_glue.cpp
CsoundMYFLTArray::CsoundMYFLTArray(int n)
{
    p = (MYFLT*) 0;
    pp = (void*) 0;
    cp = nullptr;                                             // new field init
    if (n > 0 && (size_t) n <= SIZE_MAX / sizeof(MYFLT))     // overflow guard
      pp = (void*) calloc((size_t) n, sizeof(MYFLT));        // calloc, not malloc
    if (pp) {
      p = (MYFLT*) pp;
      for (int i = 0; i < n; i++)
        ...

Three improvements in two lines:

The guard SIZE_MAX / sizeof(MYFLT) is computed at compile time (both operands are constants), so there is zero runtime cost.

Change 2 — CsoundArgVList::Insert

// FIXED — cs_glue.cpp
size_t new_cnt_sz = (cnt >= 0 ? (size_t) cnt + 1 : (size_t) 1);
if (new_cnt_sz > (size_t) INT_MAX || new_cnt_sz > SIZE_MAX / sizeof(char*) - 1)
  return;                                                     // safe early exit
new_cnt = (int) new_cnt_sz;
new_argv = (char**) calloc(new_cnt_sz + 1, sizeof(char*));

The arithmetic is now done in size_t from the start, the result is validated against both INT_MAX (because new_cnt is later used as a signed int) and SIZE_MAX / sizeof(char*), and calloc replaces malloc.

Change 3 — strncpy instead of strcpy

// BEFORE
strcpy(new_argv[i], s);

// AFTER
strncpy(new_argv[i], s, strlen(s) + 1);

This is a defence-in-depth change. Because new_argv[i] is allocated to hold exactly strlen(s) + 1 bytes, strcpy would technically be safe here — but strncpy with an explicit length makes the intent auditable and prevents future regressions if the allocation logic changes.

Prevention & Best Practices

1. Never multiply allocation arguments without an overflow pre-check

The canonical pattern in C/C++ is:

// Safe multiplication pattern
if (count > 0 && count <= SIZE_MAX / element_size) {
    ptr = malloc(count * element_size);
}

Or prefer calloc(count, element_size) which encapsulates this check and zero-initializes.

2. Prefer calloc over malloc for arrays

calloc(n, size) separates count from element size, making the intent explicit. POSIX mandates that compliant implementations detect overflow in the n * size product and return NULL rather than allocating a truncated buffer.

3. Treat all SWIG/JNI boundary inputs as untrusted

Any integer arriving from a managed language through a native binding should be validated before use in size calculations. The managed runtime may allow values that are semantically valid in Java (int up to Integer.MAX_VALUE) but dangerous in C++ size arithmetic.

4. Enable compiler and sanitizer warnings

• -fsanitize=undefined (UBSan) catches integer overflow at runtime during testing.
• -fsanitize=address (ASan) catches the resulting out-of-bounds writes.
• Clang's -Winteger-overflow and GCC's -Wstrict-overflow=5 flag suspicious arithmetic at compile time.

5. Relevant standards

• CWE-190: Integer Overflow or Wraparound
• CWE-122: Heap-based Buffer Overflow
• CWE-131: Incorrect Calculation of Buffer Size
• OWASP: Input Validation Cheat Sheet — validate size parameters before allocation

Key Takeaways

• The cast (size_t) n before a multiplication does not prevent overflow — it only changes the type in which the overflow happens. Always check n <= SIZE_MAX / sizeof(T) before multiplying.
• calloc(count, size) is safer than malloc(count * size) because it separates the two operands, zero-initializes the result, and libc implementations are required to handle overflow gracefully.
• SWIG/JNI boundary integers deserve the same distrust as network input. A Java caller can legally pass Integer.MAX_VALUE to CsoundMYFLTArray(n), and the C++ side must be prepared for it.
• Both CsoundMYFLTArray and CsoundArgVList::Insert shared the same root cause — unchecked malloc size arithmetic — demonstrating that the pattern should be audited across the entire file, not just the reported line.
• The initialization loop after allocation is the amplifier. The overflow itself produces a small allocation; the subsequent for (int i = 0; i < n; i++) loop is what turns a 7-byte buffer into a multi-gigabyte heap smash.

How Orbis AppSec Detected This

• Source: The integer n arrives from a Java caller via the SWIG binding — it is entirely attacker-controlled.
• Sink: malloc((size_t) n * sizeof(MYFLT)) at Java/cs_glue.cpp:324, where the unchecked product is used directly as the allocation size.
• Missing control: No upper-bound check on n before the multiplication; no use of calloc or a safe-multiply helper.
• CWE: CWE-190 — Integer Overflow or Wraparound.
• Fix: Added (size_t) n <= SIZE_MAX / sizeof(MYFLT) guard and replaced malloc with calloc to prevent the overflow and eliminate the undersized-allocation risk.

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 allocation arithmetic is one of the oldest classes of memory-safety bugs in C and C++, yet it continues to appear in production code because the dangerous pattern — malloc(n * sizeof(T)) — looks completely reasonable at a glance. The CsoundMYFLTArray constructor is a textbook example: a single missing bound check turned a routine array allocation into a potential heap corruption primitive accessible through the Java/SWIG interface.

The fix is small — two lines changed, one constant-time comparison added — but the security impact is significant. Developers working on any native bridge code should audit every allocation that incorporates an externally supplied count, apply the SIZE_MAX / element_size pre-check pattern, and prefer calloc over malloc for array allocations. Pair that with UBSan and ASan in your CI pipeline and you will catch the next one before it ships.

References

• CWE-190: Integer Overflow or Wraparound
• CWE-122: Heap-based Buffer Overflow
• CWE-131: Incorrect Calculation of Buffer Size
• OWASP Input Validation Cheat Sheet
• POSIX calloc specification (The Open Group)
• Semgrep rules — integer overflow
• fix: add integer overflow check in cs_glue.cpp