How buffer overflow happens in C SGX enclave memcpy and how to fix it

How Buffer Overflow Happens in C SGX Enclave memcpy and How to Fix It

Summary

A critical buffer overflow vulnerability was discovered in backend/sgx/enclave/enclave.c where the ecall_store_data function performed memcpy operations without proper bounds checking against the actual destination buffer size. An attacker could supply a malicious data_len parameter to overflow the enclave's secure storage buffer, potentially corrupting trusted execution environment memory. The fix replaces a hardcoded magic number check with a precise size comparison against the actual destination buffer, closing the vulnerability entirely.

Introduction

The backend/sgx/enclave/enclave.c file sits at the heart of a trusted execution environment: an Intel SGX enclave responsible for securely storing sensitive data. The ecall_store_data function is an ECALL — an entry point that untrusted application code calls to pass data into the enclave for protected storage. This is exactly the boundary where input validation is most critical.

But a subtle flaw in the bounds check at line 78 created a critical security gap. The check read:

if (data_len > 256) {
    return SGX_ERROR_INVALID_PARAMETER;
}

This looks reasonable at a glance. But 256 is a hardcoded magic number with no structural relationship to the actual size of the destination buffer (secure_storage[storage_count].data). If the real buffer is smaller than 256 bytes — or if it was ever resized — this check silently becomes incorrect, and a subsequent memcpy will write past the end of the buffer.

For developers writing C code that interfaces with trusted execution environments, this is a textbook example of why hardcoded size limits are dangerous.

The Vulnerability Explained

What Went Wrong

Inside ecall_store_data, data passed from untrusted code is copied into the enclave's internal secure_storage structure using memcpy. Before the copy, the code validates data_len to prevent overflows:

// VULNERABLE CODE (before fix)
if (data_len > 256) {
    return SGX_ERROR_INVALID_PARAMETER;
}

The problem: the check is against the literal integer 256, not against sizeof(secure_storage[storage_count].data). These two values may or may not be equal. If the data field in the storage struct is, say, 128 bytes, then an attacker can pass data_len = 200 — which passes the > 256 check — and the subsequent memcpy writes 200 bytes into a 128-byte buffer, overflowing by 72 bytes.

There's a second issue the fix also addresses: a data_len of 0 was not rejected. Passing zero length to memcpy is a no-op in standard C, but it can cause logic errors downstream and represents malformed input that should be explicitly rejected.

How an Attacker Could Exploit This

The attack surface here is the ECALL interface itself. An attacker (or a malicious application running outside the enclave) calls ecall_store_data with a crafted data_len that is:

• Greater than the actual buffer size (e.g., 200 when the buffer is 128 bytes)
• But less than or equal to 256 (so it passes the broken check)

The memcpy then writes attacker-controlled bytes past the end of secure_storage[storage_count].data, corrupting adjacent fields in the secure_storage array or other enclave memory. In an SGX context, this is particularly severe:

1. Enclave memory corruption can subvert the integrity guarantees SGX is designed to provide.
2. Sensitive data leakage: adjacent memory in the enclave may contain cryptographic keys, decrypted secrets, or other protected values.
3. Control flow hijacking: if a function pointer or return address is adjacent to the overflowed buffer, an attacker may redirect execution within the trusted context.

The PR notes that similar patterns exist at lines 95 and 115 of the same file, suggesting this vulnerability class was applied inconsistently throughout the enclave's input-handling code.

Real-World Impact

SGX enclaves are used specifically because they are supposed to be more trustworthy than regular application memory. A buffer overflow that corrupts enclave memory doesn't just crash the program — it can silently undermine the security model that the entire system is built on. Secrets that were supposed to be protected by the enclave could be exposed or manipulated.

The Fix

What Changed

The fix is a single-line change at line 78 of backend/sgx/enclave/enclave.c:

// BEFORE (vulnerable)
if (data_len > 256) {
    return SGX_ERROR_INVALID_PARAMETER;
}

// AFTER (fixed)
if (data_len == 0 || data_len > sizeof(secure_storage[storage_count].data)) {
    return SGX_ERROR_INVALID_PARAMETER;
}

Why This Fix Works

The new check does two things:

1. data_len == 0: Rejects zero-length inputs explicitly. This closes an edge case where zero-length copies could cause downstream logic errors.

2. data_len > sizeof(secure_storage[storage_count].data): This is the critical improvement. Instead of comparing against the magic number 256, the check now uses sizeof() to query the actual compile-time size of the destination buffer. This means:
   - If the struct is ever refactored and data changes size, the check automatically stays correct.
   - There is no gap between what the check allows and what the buffer can hold.
   - The bounds check is structurally coupled to the allocation it protects.

This is the canonical safe pattern for bounds-checking before memcpy in C.

Before vs. After

Key Takeaways

• data_len > 256 is not a safe bounds check if the actual destination buffer (secure_storage[storage_count].data) is not guaranteed to be exactly 256 bytes — use sizeof() instead.
• ECALL parameters in SGX enclaves must be treated as untrusted input, even if the enclave itself is trusted; the data crossing the boundary is not.
• Magic numbers in security-critical checks are a code smell: if the buffer size ever changes, the hardcoded check silently becomes wrong.
• Zero-length inputs should be explicitly rejected in memcpy contexts to prevent logic errors downstream.
• Lines 95 and 115 in enclave.c use similar patterns and should be reviewed for the same class of vulnerability.

How Orbis AppSec Detected This

• Source: The data_len parameter of ecall_store_data — supplied by untrusted application code calling into the SGX enclave via the ECALL interface.
• Sink: The memcpy call in backend/sgx/enclave/enclave.c:78 that copies data_len bytes into secure_storage[storage_count].data without validating against the actual buffer size.
• Missing control: The existing check (data_len > 256) used a hardcoded magic number rather than sizeof(secure_storage[storage_count].data), creating a gap between the allowed length and the actual buffer capacity.
• CWE: CWE-120 — Buffer Copy without Checking Size of Input ('Classic Buffer Overflow')
• Fix: The condition was updated to data_len == 0 || data_len > sizeof(secure_storage[storage_count].data), structurally tying the bounds check to the actual destination buffer size.

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 from unchecked memcpy calls are one of the oldest vulnerability classes in C programming — and one of the most dangerous when they occur inside a trusted execution environment like an Intel SGX enclave. The root cause here wasn't a complex algorithmic flaw; it was a single hardcoded magic number (256) that had no structural relationship to the actual destination buffer size.

The fix is equally simple: replace the magic number with sizeof(secure_storage[storage_count].data). This one-line change ensures the bounds check is always accurate, regardless of how the underlying struct evolves over time.

For developers writing C code — especially code that runs inside security-sensitive contexts like SGX enclaves — the lesson is clear: always tie your bounds checks to the actual allocation, never to a constant you think is right.