How out-of-bounds read via unchecked memcpy happens in C packet processing and how to fix it

How Out-of-Bounds Read via Unchecked memcpy Happens in C Packet Processing and How to Fix It

Introduction

The hep-tester/heptester.c file handles network packet capture and processing for the HEP (Homer Encapsulation Protocol) testing tool. Inside the callback_proto function — a pcap callback that processes every captured packet — a critical flaw at line 200 allowed an attacker to trigger an out-of-bounds memory read simply by sending a packet shorter than 18 bytes.

The vulnerable code blindly called memcpy to extract Ethernet and MPLS addresses from fixed offsets (12 and 16) within the packet buffer, without ever checking whether the packet was actually long enough to contain data at those positions. For developers working with raw network data in C, this is a textbook example of why every buffer access needs explicit length validation.

The Vulnerability Explained

Here's the vulnerable code path in callback_proto:

void callback_proto(u_char *useless, struct pcap_pkthdr *pkthdr, u_char *packet)
{
  /* Pat Callahan's patch for MPLS */
  unsigned char ethaddr[3], mplsaddr[3];

  memcpy(&ethaddr, (packet + 12), 2);
  memcpy(&mplsaddr, (packet + 16), 2);

The pcap_pkthdr structure contains caplen — the actual number of bytes captured for this packet. However, the code never checks pkthdr->caplen before reading from packet + 12 and packet + 16.

Why this is dangerous:

When memcpy(&ethaddr, (packet + 12), 2) executes, it reads 2 bytes starting at offset 12 in the packet buffer. If the packet is only, say, 5 bytes long, this reads 2 bytes from memory that doesn't belong to this packet. Similarly, memcpy(&mplsaddr, (packet + 16), 2) requires the buffer to be at least 18 bytes (offset 16 + 2 bytes).

Concrete attack scenario:

An attacker sends a crafted or truncated network packet shorter than 18 bytes to the network interface being monitored by the capture agent. When pcap delivers this packet to callback_proto:

1. pkthdr->caplen might be 4, 10, or any value less than 18
2. The packet pointer points to a buffer of only caplen bytes
3. memcpy reads past the end of this buffer
4. This results in either:
   - Information disclosure: Reading adjacent memory that may contain sensitive data from other packets or internal state
   - Crash: If the read crosses a page boundary into unmapped memory, triggering a segmentation fault and denial of service

The PR description also notes that similar patterns exist at lines 201, 202, 478, and 486, suggesting this class of bug may be systemic throughout the packet processing logic.

The Fix

The fix is elegant in its simplicity — a single bounds check added at line 200, immediately before the first memcpy:

Before:

  /* Pat Callahan's patch for MPLS */
  unsigned char ethaddr[3], mplsaddr[3];

  memcpy(&ethaddr, (packet + 12), 2);
  memcpy(&mplsaddr, (packet + 16), 2);

After:

  /* Pat Callahan's patch for MPLS */
  unsigned char ethaddr[3], mplsaddr[3];

  if (pkthdr->caplen < 18) return;
  memcpy(&ethaddr, (packet + 12), 2);
  memcpy(&mplsaddr, (packet + 16), 2);

Why 18 bytes? The highest offset accessed is 16, and 2 bytes are read from that position: 16 + 2 = 18. Any packet with caplen < 18 cannot safely provide the data these memcpy calls need.

Why return instead of error handling? In a pcap callback, returning early simply skips processing this packet. A truncated packet that doesn't contain valid Ethernet/MPLS headers isn't useful for the tool's purposes anyway — silently dropping it is the correct behavior.

The fix also includes a regression test (tests/test_invariant_heptester.c) that codifies the security invariant:

static int safe_extract_addresses(const uint8_t *packet, size_t packet_len,
                                   uint16_t *ethaddr, uint16_t *mplsaddr)
{
    /* Security invariant: must have at least 18 bytes to read offsets 12-13 and 16-17 */
    if (packet_len < 18) {
        return -1; /* Reject truncated packets */
    }
    memcpy(ethaddr, packet + 12, 2);
    memcpy(mplsaddr, packet + 16, 2);
    return 0;
}

The test exercises boundary conditions: empty buffers (0 bytes), one-byte-short buffers (17 bytes), minimum valid (18 bytes), and normal packets (100 bytes).

Prevention & Best Practices

1. Validate before every fixed-offset access:
Any time you read from a buffer at a computed or fixed offset, verify the buffer length first. This is especially critical in network code where packet lengths are attacker-controlled.

/* Pattern to follow: */
if (buffer_len < required_minimum) {
    return ERROR_CODE;
}
/* Only now is it safe to access buffer[offset] */

2. Use defensive macros for packet parsing:
Consider creating a bounds-checked read macro:

#define SAFE_READ(dst, src, offset, len, buflen) \
    do { if ((offset) + (len) > (buflen)) return -1; \
         memcpy((dst), (src) + (offset), (len)); } while(0)

3. Enable AddressSanitizer during development:
Compile with -fsanitize=address during testing. ASan would immediately flag this out-of-bounds read when processing a short test packet.

4. Audit all similar patterns:
As noted in the PR, lines 201, 202, 478, and 486 contain similar patterns. After fixing one instance, systematically review all memcpy calls that use fixed offsets from packet buffers.

5. Fuzz test packet processing:
Use AFL or libFuzzer to generate packets of various lengths, including very short ones. This would catch this class of bug automatically.

Key Takeaways

• Every memcpy from a network packet buffer at a fixed offset needs a preceding length check against pkthdr->caplen — this is non-negotiable in pcap callback functions.
• The minimum buffer size is the highest offset plus the number of bytes read: offset 16 + 2 bytes = 18 bytes minimum for callback_proto.
• Truncated packets are not exceptional — they occur naturally (capture snaplen limits, fragmentation) and maliciously (crafted attacks). Code must handle them gracefully.
• A single missing bounds check in heptester.c:200 exposed the entire capture agent to remote crash or memory disclosure from any host that could send packets to the monitored interface.
• Regression tests should encode the security invariant (minimum packet length) independently of the implementation, so refactors don't silently reintroduce the bug.

How Orbis AppSec Detected This

• Source: Network packet data received via pcap capture (u_char *packet parameter in callback_proto)
• Sink: memcpy(&ethaddr, (packet + 12), 2) and memcpy(&mplsaddr, (packet + 16), 2) at hep-tester/heptester.c:200-201
• Missing control: No validation that pkthdr->caplen >= 18 before accessing packet data at offsets 12 and 16
• CWE: CWE-125 (Out-of-bounds Read)
• Fix: Added if (pkthdr->caplen < 18) return; before the memcpy calls to reject packets too short to contain valid Ethernet/MPLS header data

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

This vulnerability demonstrates a fundamental truth about C network programming: the data you receive from the network is adversarial. Every byte offset, every memcpy, every array index into a packet buffer is a potential out-of-bounds access if you haven't validated the buffer length first. The fix — a single if statement — is trivial, but the consequences of its absence are severe: remote crash or information leakage from any host that can put packets on the wire.

If you're writing pcap callbacks or any packet processing code in C, audit every fixed-offset access today. The pattern is simple: check length, then read. Never the other way around.

References

• CWE-125: Out-of-bounds Read
• OWASP Buffer Overflow
• pcap_pkthdr documentation (caplen field)
• Semgrep rules for buffer overflow
• fix: add bounds check before memcpy in heptester.c