DEV Community

Morgan Ma
Morgan Ma

Posted on

The Signed Length Passed the Maximum Check

A signed length is never a safe container size. I relearned that on a twenty line parser. The tests stayed green until one hostile frame arrived.

Did I need a cluster outage to see it? No. I needed four bytes and a printer. This write-up is that lab, step by step.

The symptom

The process died inside a call to std::vector::resize. AddressSanitizer stayed quiet on the happy path. Nothing overran a live buffer in those tests.

The allocator received a gigantic count instead. I printed the size before the crash returned. The number looked like a size_t with every bit set.

Does that sound like a heap leak to you? It was a type conversion in disguise. The copy never ran far enough to matter.

Why the happy tests lied

My fixtures used lengths of 0, 1, 16, and 1024. I also added one oversize value above kMax. Did any fixture set the high bit? Never.

So the branch len > kMax looked complete. It was not a complete range check. Negative lengths fail that upper bound test.

Then the cast into size_t made them huge. The remaining-bytes check used signed addition. That check also let -1 through.

The baseline reader

I kept the reader tiny on purpose. Four header bytes, then a payload. I assumed a little-endian lab host.

This is the broken core. Treat it as a labeled lab, not production code.

// lab_frame.cpp — baseline, intentionally broken
#include <cstdint>
#include <cstring>
#include <iostream>
#include <iterator>
#include <optional>
#include <vector>

constexpr int32_t kMax = 1 << 20;

std::optional<std::vector<uint8_t>> read_frame(
    const uint8_t* data, size_t n) {
  if (n < 4) {
    return std::nullopt;
  }

  int32_t len = 0;
  std::memcpy(&len, data, sizeof(len));

  if (len > kMax) {
    return std::nullopt;
  }

  // Signed add. Negative len can pass.
  if (4 + len > static_cast<int32_t>(n)) {
    return std::nullopt;
  }

  std::vector<uint8_t> payload;
  payload.resize(static_cast<size_t>(len));
  std::memcpy(payload.data(), data + 4, payload.size());
  return payload;
}

int main() {
  std::vector<uint8_t> buf(
      (std::istreambuf_iterator<char>(std::cin)),
      std::istreambuf_iterator<char>());
  auto out = read_frame(buf.data(), buf.size());
  if (!out) {
    std::cerr << "reject\n";
    return 1;
  }
  std::cout << "payload=" << out->size() << "\n";
  return 0;
}
Enter fullscreen mode Exit fullscreen mode

Four header bytes sit in host order. The payload follows immediately. No version field exists in this lab.

Build the lab

Use a sanitizer build. Do not start with -O2.

g++ -std=c++17 -O0 -g -Wall -Wextra \
  -fsanitize=address,undefined \
  lab_frame.cpp -o lab_frame
Enter fullscreen mode Exit fullscreen mode

Feed it a normal frame first. Four bytes of length one, then one byte.

python3 -c "import sys; sys.stdout.buffer.write(bytes([1,0,0,0, 65]))" \
  | ./lab_frame
Enter fullscreen mode Exit fullscreen mode

That path prints payload=1 and I trusted it. The green output hid the missing lower bound. I had not yet sent a hostile header.

Now feed the hostile header. All 0xFF bytes. No payload body follows.

python3 -c "import sys; sys.stdout.buffer.write(bytes([255,255,255,255]))" \
  | ./lab_frame
Enter fullscreen mode Exit fullscreen mode

What happens on your machine after that pipe? I got std::bad_alloc or a sanitizer abort. The happy path never produced that failure.

What I blamed first

I walked through the wrong rooms first. That wasted time is the actual lesson. Tools failed after the predicate failed.

  1. I blamed endianness on a little-endian host. The fixture already matched the lab host layout. A swap bug would still show small positive lengths.
  2. I blamed memcpy of int32_t as alignment UB. The buffer came from std::vector storage. That alignment worry was mostly noise here.
  3. I blamed AddressSanitizer for missing a wild copy. There was no live copy yet. resize died before memcpy ran.
  4. I blamed the test runner for swallowing aborts. The runner never saw the hostile frame at all. I had not added that row.

Notice the pattern in those four turns? I debugged the tools around the function. I did not debug the boolean predicates inside it.

The frame that told the truth

I printed len in decimal and hex. The hostile header decoded as -1 on this host. 0xFFFFFFFF as two's complement int32_t is -1.

Why did the guard miss that value? -1 > 1048576 is false. An upper bound is not a range.

Then came the second lie in the same function. 4 + len became 3. 3 > 4 is false, so the body check passed. The function then called resize.

static_cast<size_t>(-1) is not -1 in any useful sense. It is a mountain of bytes. The vector asked the allocator for that mountain.

Root cause

Three facts sat in one function. I treated them as one fact. They were not one fact.

  1. The header type was signed.
  2. The upper bound check had no lower bound.
  3. The remaining-length check added signed values.

Any one of those can fail closed in isolation. Together they failed open on 0xFFFFFFFF. Then size_t hid the sign from resize.

This was not a clever exploit in any form. It was a missing row in a table. The compiler did exactly what I typed.

The debugging sequence I now reuse

I keep this order now. The sequence looks boring, and that is the point. Fancy stepping comes last.

  1. Freeze the input bytes. Print them in hex. Do not pretty-print the struct first.
  2. Print the decoded header fields with their real types. Print signed and unsigned views together.
  3. Print each predicate with both operands. Force the types into the log line.
  4. Only then step into resize, memcpy, or a parser loop. The crash site is often downstream.
  5. Add the failing frame to a table. Do not patch by instinct after one abort.

Here is the log I wish I had first. It is not pretty. It is local.

std::cerr << "n=" << n
          << " len=" << len
          << " len_u=" << static_cast<uint32_t>(len)
          << " pred_max=" << (len > kMax)
          << " pred_need=" << (4 + len)
          << " n_i32=" << static_cast<int32_t>(n)
          << "\n";
Enter fullscreen mode Exit fullscreen mode

The log is ugly and that is fine. It is fast enough for a parser lab. Compare len and len_u on the same line.

The bug stops being a story at that point. It becomes two numbers and a boolean. You can keep the debugger closed for a minute.

The matrix that would have caught it

I now run this table before I trust a frame reader. Each row is one process. Each process gets a fresh stdin.

name header bytes payload expect
empty none none reject
truncated header 01 00 none reject
zero 00 00 00 00 none accept 0
one 01 00 00 00 41 accept 1
max 00 00 10 00 kMax bytes accept kMax
max plus 01 00 10 00 none reject
all ff FF FF FF FF none reject
high bit 00 00 00 80 none reject
truncated body 02 00 00 00 41 reject

The last three rows are the whole point. I did not have them, and the first draft did not add them. Positive tutorials rarely emit a high bit.

How do you run a single row then? Same binary. Different stdin. Assert the exit code after the pipe.

python3 -c "import sys; sys.stdout.buffer.write(bytes([0,0,0,128]))" \
  | ./lab_frame ; echo exit:$?
Enter fullscreen mode Exit fullscreen mode

0x80000000 is INT32_MIN on this little-endian layout. That INT32_MIN row is not optional here. Zero, one, and kMax never touch it.

The fix

Reject the sign before any arithmetic. Compare remaining bytes without signed add. Copy only after both checks pass.

std::optional<std::vector<uint8_t>> read_frame(
    const uint8_t* data, size_t n) {
  if (n < 4) {
    return std::nullopt;
  }

  int32_t len = 0;
  std::memcpy(&len, data, sizeof(len));

  if (len < 0 || len > kMax) {
    return std::nullopt;
  }

  const size_t need = static_cast<size_t>(len);
  if (n - 4 < need) {
    return std::nullopt;
  }

  std::vector<uint8_t> payload(need);
  if (need != 0) {
    std::memcpy(payload.data(), data + 4, need);
  }
  return payload;
}
Enter fullscreen mode Exit fullscreen mode

Why n - 4 < need after the n < 4 guard? I already know n >= 4. Subtraction does not wrap on that path. Adding a huge need to 4 still can.

I also skip memcpy when need is zero. That avoids a null pointer discussion on some libraries. Empty payloads are legal in this lab.

Is the header still host-endian after the fix? Yes. A real protocol needs an explicit endian decode later. This lab isolates the signed-size mistake only.

What I asked a model to do

I wanted a second pair of eyes on the first draft. I did not want a story about agents. I wanted predicates I could print.

Disclosure: This article was prepared as part of MonkeyCode's product outreach. I used MonkeyCode's free model access and free server option as a scratch box for this lab. The model mirrored my green tests and restated len > kMax. It did not invent len < 0.

That miss is useful as a review lesson. Models copy the distribution you feed them. Hostile rows are rare in parser tutorials, so the predicate stays half-done.

The scratch box compiled what I pasted. It did not invent the table. I still had to type the 0xFF frame by hand.

A second miss I almost shipped

After the sign fix, I still had a sharp edge. int32_t addition of 4 + len is undefined when len is huge and signed. I removed that add instead of narrowing it.

I also almost compared len to n with mixed types. Usual arithmetic conversions would promote a negative len to a huge size_t. Then len > n could look true on poison input.

That rejection is not a specification I can trust. It is an accidental conversion on some paths. Write the checks in one type and cast on purpose.

Log the cast beside the predicate. If you cannot see the type, you cannot see the branch. Mixed comparisons belong in the same bucket as missing lower bounds.

Limitations

This lab assumes a little-endian host and a four-byte header. It does not prove a wire protocol. It does not fuzz a full parser with nested fields.

Sanitizers catch many wild copies. They do not catch every giant allocation. bad_alloc is not a green test, so treat it as a red row.

I did not measure throughput on this reader. I did not claim a quota, a model name, or a hardware size. Those details change, and I will not invent them here.

The matrix is not a fuzzer. A fuzzer would mutate the header and the body. Use one after the table is green, not before the predicates exist.

Who should not use this approach

Skip this pattern if you already parse with a checked slice type. Skip it if your header is unsigned and you still skip a max. An unsigned length still needs a cap.

Do not use a chat window as your only review. Do not deploy a frame reader with only positive fixtures. Do not silence bad_alloc and call that resilience.

If you cannot print the raw header bytes, stop and get a dump first. Hex before structs. Types before copies.

What I keep

Every length prefix is a trust boundary. A maximum check is not a range check. A green test suite can still be a signed-length suite.

I start with hex now. Then types. Then predicates. Then the copy into the vector.

Clone the matrix before you trust the parser. Then add one uglier frame than mine.

Top comments (0)