trevdatastreams's picture
Add verified Apache Avro C++ varint PoC
a706ef6 verified
|
Raw
History Blame Contribute Delete
4.69 kB

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:

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

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:

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:

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:

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.