report-draft.md

# Title

Ten-byte Avro datum causes infinite loop and undefined shift in C++ legacy
`Reader`

# Target and version

- Target: Apache Avro C++
- Format: Avro raw binary datum (`.avro`)
- Commit: `44f7aa35a6b90ac7d39ef0e68dad706561235941`
- Public APIs: `Reader`, `ValidatingReader`, and `ResolvingReader`
- Primary classification: CWE-835, loop with unreachable exit condition
- Secondary classification: CWE-758, reliance on undefined behavior

# Summary

Apache Avro C++'s legacy public Reader path ignores the result of
`BufferReader::read()` while decoding variable-length integers and never
checks the shift count.

If an untrusted binary datum ends while the final byte has its continuation
bit set, `read()` returns `false` but leaves that byte in `val`.
`readVarInt()` ignores the failure, sees the continuation bit again, and loops
forever after input EOF. A 10-byte file is sufficient to pin one CPU core.

A terminated 11-byte varint reaches the related unchecked-shift flaw:
UBSan reports a shift exponent of 70 for a 64-bit value. Without UBSan, the
legacy Reader silently returns a bogus negative value. The modern public
`BinaryDecoder` correctly rejects the same bytes with `Invalid Avro varint`.

# Root cause

At `lang/c++/include/avro/Reader.hh:167-176`:

```cpp
uint64_t readVarInt() {
    uint64_t encoded = 0;
    uint8_t val = 0;
    int shift = 0;
    do {
        reader_.read(val);
        uint64_t newBits = static_cast<uint64_t>(val & 0x7f) << shift;
        encoded |= newBits;
        shift += 7;
    } while (val & 0x80);
    return encoded;
}
```

Two validation results are missing:

1. `BufferReader::read(val)` returns `false` at EOF, but the result is ignored.
2. `shift` is not rejected before it reaches or exceeds 64.

The modern implementation at
`lang/c++/impl/BinaryDecoder.cc:205-215` performs the necessary
`shift >= 64` check and obtains bytes through an input method that throws at
EOF.

This vulnerable Reader is not test-only. It is the implementation behind the
public `Reader` and `ValidatingReader` aliases. `ResolvingReader` stores a
legacy `Reader` internally and exposes it through the public translating parse
path.

# Reproduction

```bash
git clone https://github.com/apache/avro.git
cmake -S avro/lang/c++ -B avro-build \
  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
  -DCMAKE_CXX_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -g" \
  -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined"
cmake --build avro-build --target avrocpp_s -j

AVRO_SRC=/path/to/avro/lang/c++ \
AVRO_BUILD=/path/to/avro-build \
./reproduce.sh
```

Observed in three consecutive runs:

```text
Reader.hh:173:66: runtime error: shift exponent 70 is too large
trigger: modern BinaryDecoder rejected input: Invalid Avro varint
trigger: legacy Reader decoded -4647998506761461825
TIMEOUT: legacy Reader did not return after input EOF
PASS: truncated input caused deterministic non-termination (exit 124)
```

The 10-byte non-termination fixture is:

```text
81 81 81 81 81 81 81 81 81 81
```

The alarm in `poc_hang.cpp` terminates the isolated harness after two seconds
and returns exit 124. Without that harness alarm, the legacy Reader continues
running.

# Security impact

Applications using Apache Avro's C++ legacy parsing API can be made to consume
unbounded CPU by loading a 10-byte attacker-controlled datum. Services that
parse uploaded or remotely obtained Avro data can lose one worker or CPU core
per concurrent malicious input.

The separately supplied overlong fixture also demonstrates undefined behavior
and silent parser disagreement: a malformed value rejected by the modern
decoder becomes an attacker-influenced negative value in the legacy decoder.
That can corrupt application decisions when the decoded long is trusted.

Suggested CVSS 3.1:
`AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:L/A:H`.

# Prior art

A fresh pre-submission scan found no exact Hugging Face or GitHub report for
the legacy Reader's ignored EOF result or unbounded shift.

AVRO-4228 is adjacent but distinct. It concerns negative array block counts in
the modern `BinaryDecoder::arrayNext()` path. It does not address malformed
varint termination, `BufferReader::read()` failure, or this legacy Reader.

# Suggested fix

Mirror the modern decoder's validation:

```cpp
do {
    if (shift >= 64) {
        throw Exception("Invalid Avro varint");
    }
    if (!reader_.read(val)) {
        throw Exception("EOF reached while decoding Avro varint");
    }
    encoded |= static_cast<uint64_t>(val & 0x7f) << shift;
    shift += 7;
} while (val & 0x80);
```

Add regression tests for a valid 10-byte long, an 11-byte overlong value, and a
truncated continuation sequence.