diff --git a/lang/c++/CMakeLists.txt b/lang/c++/CMakeLists.txt index a88bc77f096..3e7c5ff1610 100644 --- a/lang/c++/CMakeLists.txt +++ b/lang/c++/CMakeLists.txt @@ -221,6 +221,7 @@ if (AVRO_BUILD_TESTS) unittest (LargeSchemaTests) unittest (CodecTests) unittest (StreamTests) + unittest (AvailableBytesTests) unittest (SpecificTests) unittest (DataFileTests) unittest (JsonTests) diff --git a/lang/c++/impl/BinaryDecoder.cc b/lang/c++/impl/BinaryDecoder.cc index b334de7cf5d..564853fb62e 100644 --- a/lang/c++/impl/BinaryDecoder.cc +++ b/lang/c++/impl/BinaryDecoder.cc @@ -19,12 +19,40 @@ #include "Decoder.hh" #include "Exception.hh" #include "Zigzag.hh" +#include +#include +#include +#include #include namespace avro { using std::make_shared; +// Structural cap on the number of elements to skip in an array or map (an +// overflow / defense-in-depth guard). Mirrors the read-path limit in Generic.cc +// and matches the value used by the other Avro SDKs (Integer.MAX_VALUE - 8). +static constexpr int64_t kDefaultMaxCollectionStructural = 2147483639; + +// Returns the structural collection cap. It can be overridden by the +// AVRO_MAX_COLLECTION_ITEMS environment variable (a non-negative integer), +// matching the read path and the other SDKs so a single knob configures all of +// them; an invalid or unset value uses the default. +static int64_t maxCollectionStructural() { + const char *env = std::getenv("AVRO_MAX_COLLECTION_ITEMS"); + if (env != nullptr && *env != '\0') { + char *end = nullptr; + errno = 0; + long long value = std::strtoll(env, &end, 10); + // Ignore an overflowing value (errno == ERANGE) rather than accepting a + // saturated LLONG_MAX, which would effectively remove the structural cap. + if (errno == 0 && *end == '\0' && value >= 0) { + return static_cast(value); + } + } + return kDefaultMaxCollectionStructural; +} + class BinaryDecoder : public Decoder { StreamReader in_; @@ -51,9 +79,15 @@ class BinaryDecoder : public Decoder { size_t decodeUnionIndex() final; int64_t doDecodeLong(); + size_t decodeIndex(); size_t doDecodeItemCount(); size_t doDecodeLength(); + void checkAvailableBytes(size_t len); void drain() final; + + int64_t bytesRemaining() const final { + return in_.remainingBytes(); + } }; DecoderPtr binaryDecoder() { @@ -109,12 +143,25 @@ size_t BinaryDecoder::doDecodeLength() { return len; } +void BinaryDecoder::checkAvailableBytes(size_t len) { + // Reject a declared length that exceeds the data actually available before + // allocating for it, to guard against an out-of-memory attack from a + // malicious or truncated input. Only enforced when the stream can report + // how many bytes remain. + int64_t remaining = in_.remainingBytes(); + if (remaining >= 0 && static_cast(len) > static_cast(remaining)) { + throw Exception( + "Length {} exceeds the {} bytes available in the stream", len, remaining); + } +} + void BinaryDecoder::drain() { in_.drain(false); } void BinaryDecoder::decodeString(std::string &value) { size_t len = doDecodeLength(); + checkAvailableBytes(len); value.resize(len); if (len > 0) { in_.readBytes(const_cast( @@ -125,11 +172,13 @@ void BinaryDecoder::decodeString(std::string &value) { void BinaryDecoder::skipString() { size_t len = doDecodeLength(); + checkAvailableBytes(len); in_.skipBytes(len); } void BinaryDecoder::decodeBytes(std::vector &value) { size_t len = doDecodeLength(); + checkAvailableBytes(len); value.resize(len); if (len > 0) { in_.readBytes(value.data(), len); @@ -138,6 +187,7 @@ void BinaryDecoder::decodeBytes(std::vector &value) { void BinaryDecoder::skipBytes() { size_t len = doDecodeLength(); + checkAvailableBytes(len); in_.skipBytes(len); } @@ -153,7 +203,7 @@ void BinaryDecoder::skipFixed(size_t n) { } size_t BinaryDecoder::decodeEnum() { - return static_cast(doDecodeLong()); + return decodeIndex(); } size_t BinaryDecoder::arrayStart() { @@ -163,8 +213,28 @@ size_t BinaryDecoder::arrayStart() { size_t BinaryDecoder::doDecodeItemCount() { auto result = doDecodeLong(); if (result < 0) { - doDecodeLong(); - return static_cast(-(result + 1)) + 1; + // INT64_MIN cannot be negated in int64_t (it would overflow); reject it + // rather than propagating 2^63 as an item count that inevitably fails a + // huge allocation downstream. + if (result == INT64_MIN) { + throw Exception("Invalid negative block count: {}", result); + } + int64_t blockSize = doDecodeLong(); + if (blockSize < 0) { + // The byte-size that follows a negative block count is a byte count + // and must be non-negative; reject malformed input here too (as + // skipArray already does) so arrayStart()/mapStart() fail fast. + throw Exception("Invalid negative block byte-size: {}", blockSize); + } + result = -result; + } + // On builds where size_t is narrower than int64_t (e.g. 32-bit), reject a + // count that would truncate on the cast -- otherwise a huge block could wrap + // to a small one and bypass the downstream structural caps. + if constexpr (sizeof(size_t) < sizeof(int64_t)) { + if (static_cast(result) > std::numeric_limits::max()) { + throw Exception("Block count {} exceeds the maximum supported size", result); + } } return static_cast(result); } @@ -177,9 +247,44 @@ size_t BinaryDecoder::skipArray() { for (;;) { auto r = doDecodeLong(); if (r < 0) { - auto n = static_cast(doDecodeLong()); - in_.skipBytes(n); + auto byteSize = doDecodeLong(); + if (byteSize < 0) { + // A negative block byte-size would convert to a huge size_t and + // drive an unbounded skip; reject it. + throw Exception("Invalid negative block size: {}", byteSize); + } + // Reject a byte-size that would truncate on the cast (32-bit builds) + // and one that exceeds the bytes remaining, so a truncated block is + // not silently skipped past EOF by the memory-backed skip(). + if constexpr (sizeof(size_t) < sizeof(int64_t)) { + if (static_cast(byteSize) > std::numeric_limits::max()) { + throw Exception("Block size {} exceeds the maximum supported size", byteSize); + } + } + checkAvailableBytes(static_cast(byteSize)); + in_.skipBytes(static_cast(byteSize)); } else { + // Bound the block count: skipping a huge block of zero-byte elements + // would otherwise loop unboundedly (a CPU exhaustion) even though it + // reads/allocates nothing. The decoder has no element schema here, so + // apply the structural cap (AVRO_MAX_COLLECTION_ITEMS, default + // Integer.MAX_VALUE - 8). Read the limit each call so a runtime + // change to the environment variable is honoured, matching Generic.cc. + const int64_t structural = maxCollectionStructural(); + if (r > structural) { + throw Exception( + "Cannot skip a collection of more than {} elements; " + "set AVRO_MAX_COLLECTION_ITEMS if this is legitimate", + structural); + } + // On builds where size_t is narrower than int64_t (e.g. 32-bit), + // reject a count that would truncate on the cast, matching + // doDecodeItemCount so a huge block can't wrap to a small one. + if constexpr (sizeof(size_t) < sizeof(int64_t)) { + if (static_cast(r) > std::numeric_limits::max()) { + throw Exception("Block count {} exceeds the maximum supported size", r); + } + } return static_cast(r); } } @@ -198,7 +303,7 @@ size_t BinaryDecoder::skipMap() { } size_t BinaryDecoder::decodeUnionIndex() { - return static_cast(doDecodeLong()); + return decodeIndex(); } int64_t BinaryDecoder::doDecodeLong() { @@ -217,4 +322,21 @@ int64_t BinaryDecoder::doDecodeLong() { return decodeZigzag64(encoded); } +// Decode an enum ordinal or union branch index. Both are wire longs that the +// callers cast to size_t; validate the decoded value here so a negative index +// cannot wrap to a huge size_t and a value beyond size_t (possible where size_t +// is narrower than int64_t, e.g. 32-bit builds) cannot truncate into a +// spuriously in-range branch. The concrete bound (against the enum/union size) +// is still enforced by the caller. +size_t BinaryDecoder::decodeIndex() { + int64_t index = doDecodeLong(); + if (index < 0) { + throw Exception("Invalid negative index: {}", index); + } + if (static_cast(index) > std::numeric_limits::max()) { + throw Exception("Index {} is out of range", index); + } + return static_cast(index); +} + } // namespace avro diff --git a/lang/c++/impl/Generic.cc b/lang/c++/impl/Generic.cc index 1535c604be7..9fe9f29b6ba 100644 --- a/lang/c++/impl/Generic.cc +++ b/lang/c++/impl/Generic.cc @@ -17,6 +17,11 @@ */ #include "Generic.hh" +#include +#include +#include +#include +#include #include namespace avro { @@ -25,6 +30,167 @@ using std::ostringstream; using std::string; using std::vector; +// Minimum number of bytes a single value of the given schema node can occupy on +// the wire. Used to reject an array/map block count that could not be backed by +// the bytes remaining. A type that can encode to zero bytes (null) returns 0, +// which disables the collection check for it (so an array of nulls is not +// falsely rejected). A depth limit breaks self-referencing (symbolic) schemas. +static int64_t minBytesPerElement(const NodePtr &node, int depth) { + if (!node) { + return 0; + } + switch (node->type()) { + case AVRO_NULL: + return 0; + case AVRO_FLOAT: + return 4; + case AVRO_DOUBLE: + return 8; + case AVRO_FIXED: { + // fixedSize() is a size_t; clamp to int64_t so a huge fixed size + // cannot wrap negative. + return static_cast(std::min( + node->fixedSize(), + static_cast(std::numeric_limits::max()))); + } + case AVRO_RECORD: { + if (depth > 64) { + // Purely a recursion (stack-overflow) safety net for a + // pathologically deep schema. A truly cyclic schema never + // reaches this: a self-reference is an AVRO_SYMBOLIC node, + // handled by the default case below (returning 1 without + // following the link), so recursion always terminates. This + // guard therefore only trips on a genuinely deep *acyclic* + // record, whose true minimum can still be 0 (e.g. a long chain + // of records whose only leaves are null). Return 0 rather than + // over-estimating, so a valid array/map of such elements is not + // falsely rejected; 0 is always a valid lower bound. + return 0; + } + int64_t total = 0; + for (size_t i = 0; i < node->leaves(); ++i) { + int64_t fieldMin = minBytesPerElement(node->leafAt(i), depth + 1); + // Saturate rather than overflow: a wrapped (negative) total + // would disable the collection check. + if (fieldMin > std::numeric_limits::max() - total) { + return std::numeric_limits::max(); + } + total += fieldMin; + } + return total; + } + default: + // boolean, int, long, bytes, string, enum, union, array, map and + // symbolic references all encode to at least one byte. + return 1; + } +} + +// Default maximum number of zero-byte-encoded collection elements (e.g. an +// array of nulls) to allocate from a single decode. Such elements consume no +// input, so ensureCollectionAvailable cannot bound their count; without a cap a +// tiny payload can declare a huge block count and exhaust memory. Overridable +// via the AVRO_MAX_COLLECTION_ITEMS environment variable. +static constexpr int64_t kDefaultMaxCollectionItems = 10000000; + +// Structural cap on the number of elements in any array or map (an overflow / +// defense-in-depth guard). It matches the value used by the other Avro SDKs +// (Integer.MAX_VALUE - 8); non-zero-byte elements are also bounded by the bytes +// remaining. +static constexpr int64_t kDefaultMaxCollectionStructural = 2147483639; + +// Upper bound on how many elements a collection container is grown by in a +// single step while decoding. The container still grows to hold every element +// that is actually read; this only prevents resizing to the full (possibly +// attacker-declared) block count up front, before any element is decoded. That +// matters most for stream sources, where the bytes-remaining check cannot bound +// the declared count against the input, so a single up-front resize could +// allocate a huge container before the truncated stream is detected. +static constexpr size_t kMaxCollectionPrealloc = 1024; + +struct CollectionLimits { + int64_t zeroByte; + int64_t structural; +}; + +// AVRO_MAX_COLLECTION_ITEMS, when a non-negative integer, overrides both limits +// to that value; otherwise zero-byte elements use the tighter default and all +// collections use the structural default. +static CollectionLimits collectionLimits() { + const char *env = std::getenv("AVRO_MAX_COLLECTION_ITEMS"); + if (env != nullptr && *env != '\0') { + char *end = nullptr; + errno = 0; + long long value = std::strtoll(env, &end, 10); + // Ignore an overflowing value (errno == ERANGE) rather than accepting a + // saturated LLONG_MAX, which would effectively disable the caps. + if (errno == 0 && *end == '\0' && value >= 0) { + return {static_cast(value), static_cast(value)}; + } + } + return {kDefaultMaxCollectionItems, kDefaultMaxCollectionStructural}; +} + +// Reject a collection (array or map) block that could drive an unbounded +// allocation, before resizing for it. A block whose declared element count +// could not be backed by the bytes actually remaining is rejected (only when +// the per-element minimum is positive and the decoder can report the bytes +// remaining); and every collection is bounded by the structural cap, which also +// covers zero-byte elements and decoders that cannot report the bytes remaining. +// The comparisons divide/subtract to avoid overflow. +static void ensureCollectionAvailable(Decoder &d, size_t existing, size_t count, int64_t minBytes) { + if (count == 0) { + return; + } + if (minBytes > 0) { + int64_t remaining = d.bytesRemaining(); + if (remaining >= 0 && + static_cast(count) > + static_cast(remaining) / static_cast(minBytes)) { + throw Exception( + "Collection claims {} elements with at least {} bytes each, " + "but only {} bytes are available", + count, minBytes, remaining); + } + } + const int64_t structural = collectionLimits().structural; + const uint64_t limit = static_cast(structural); + if (static_cast(count) > limit || + static_cast(existing) > limit - static_cast(count)) { + throw Exception( + "Cannot read a collection of more than {} elements; " + "set AVRO_MAX_COLLECTION_ITEMS if this is legitimate", + structural); + } +} + +// Reject a collection of zero-byte elements (e.g. null) whose cumulative count +// exceeds the configured limit. These elements consume no input, so they cannot +// be bounded by the bytes remaining; the count is the only signal. +static void ensureZeroByteCollectionWithinLimit(size_t existing, size_t count) { + const int64_t zeroByte = collectionLimits().zeroByte; + const uint64_t limit = static_cast(zeroByte); + // Compare without adding, so existing + count cannot overflow. + if (static_cast(count) > limit || + static_cast(existing) > limit - static_cast(count)) { + throw Exception( + "Cannot read a collection of more than {} zero-byte elements; " + "set AVRO_MAX_COLLECTION_ITEMS if this is legitimate", + zeroByte); + } +} + +// Guard against size_t overflow / an over-large request when growing a +// collection container by `count` elements before calling resize(). +template +static void ensureCanGrow(const Container &c, size_t count) { + if (count > c.max_size() - c.size()) { + throw Exception( + "Collection block count {} exceeds the maximum container size", + count); + } +} + typedef vector bytes; void GenericContainer::assertType(const NodePtr &schema, Type type) { @@ -105,11 +271,38 @@ void GenericReader::read(GenericDatum &datum, Decoder &d, bool isResolving) { const NodePtr &nn = v.schema()->leafAt(0); r.resize(0); size_t start = 0; + // Only when not resolving: the datum schema then matches the wire + // schema, so minBytesPerElement is a true lower bound. Under + // resolution the wire (writer) type may be smaller than the datum + // (reader) type, which would over-estimate and reject valid data. + int64_t trueMin = minBytesPerElement(nn, 0); + // Under resolution the on-wire (writer) element can be zero bytes + // even when the reader element is not (e.g. reader-only fields filled + // from defaults), so the bytes check is disabled and we cannot tell + // whether an element is zero-byte on the wire. Apply the tighter + // zero-byte cap conservatively in that case, so the up-front resize + // cannot be driven past it. + bool zeroByte = isResolving || trueMin == 0; + int64_t minBytes = isResolving ? 0 : trueMin; for (size_t m = d.arrayStart(); m != 0; m = d.arrayNext()) { - r.resize(r.size() + m); - for (; start < r.size(); ++start) { - r[start] = GenericDatum(nn); - read(r[start], d, isResolving); + ensureCollectionAvailable(d, r.size(), m, minBytes); + // Zero-byte elements are not bounded by the bytes check, so cap + // their cumulative count (r.size() is the count so far). + if (zeroByte) { + ensureZeroByteCollectionWithinLimit(r.size(), m); + } + ensureCanGrow(r, m); + // Grow on demand in bounded steps rather than resizing to the + // full block count up front, so a huge declared count on a stream + // cannot allocate a giant container before any element is read. + for (size_t remaining = m; remaining != 0;) { + size_t step = std::min(remaining, kMaxCollectionPrealloc); + r.resize(r.size() + step); + for (; start < r.size(); ++start) { + r[start] = GenericDatum(nn); + read(r[start], d, isResolving); + } + remaining -= step; } } } break; @@ -119,12 +312,32 @@ void GenericReader::read(GenericDatum &datum, Decoder &d, bool isResolving) { const NodePtr &nn = v.schema()->leafAt(1); r.resize(0); size_t start = 0; + // Map keys are strings (>= 1 byte length prefix) plus the value. + // Saturate the +1 so a maxed-out value minimum cannot wrap. + int64_t valuesMin = minBytesPerElement(nn, 0); + // A map entry always includes a >= 1 byte key, so it is never a + // zero-byte element and even under resolution the count is bounded by + // the bytes remaining (each entry consumes at least the key). Use 1 + // when resolving (the value type may differ), else the key + value. + int64_t minBytes = isResolving + ? 1 + : (valuesMin < std::numeric_limits::max() + ? valuesMin + 1 + : valuesMin); for (size_t m = d.mapStart(); m != 0; m = d.mapNext()) { - r.resize(r.size() + m); - for (; start < r.size(); ++start) { - d.decodeString(r[start].first); - r[start].second = GenericDatum(nn); - read(r[start].second, d, isResolving); + ensureCollectionAvailable(d, r.size(), m, minBytes); + ensureCanGrow(r, m); + // Grow on demand in bounded steps rather than resizing to the + // full block count up front (see the array case above). + for (size_t remaining = m; remaining != 0;) { + size_t step = std::min(remaining, kMaxCollectionPrealloc); + r.resize(r.size() + step); + for (; start < r.size(); ++start) { + d.decodeString(r[start].first); + r[start].second = GenericDatum(nn); + read(r[start].second, d, isResolving); + } + remaining -= step; } } } break; diff --git a/lang/c++/impl/Stream.cc b/lang/c++/impl/Stream.cc index 1ca5c346466..a58d5a37894 100644 --- a/lang/c++/impl/Stream.cc +++ b/lang/c++/impl/Stream.cc @@ -84,6 +84,17 @@ class MemoryInputStream : public InputStream { size_t byteCount() const final { return cur_ * chunkSize_ + curLen_; } + + int64_t remainingBytes() const final { + // Total capacity across all chunks: full chunks plus the (partial) + // last chunk, minus what has already been consumed. Widen to int64_t + // before multiplying so the arithmetic cannot overflow size_t. + int64_t total = (size_ == 0) + ? 0 + : static_cast(size_ - 1) * static_cast(chunkSize_) + static_cast(available_); + int64_t consumed = static_cast(cur_) * static_cast(chunkSize_) + static_cast(curLen_); + return total - consumed; + } }; class MemoryInputStream2 : public InputStream { @@ -119,6 +130,13 @@ class MemoryInputStream2 : public InputStream { size_t byteCount() const final { return curLen_; } + + int64_t remainingBytes() const final { + // Subtract in int64_t: if an invariant were ever violated (curLen_ > + // size_), doing the subtraction in unsigned size_t would underflow to a + // huge value and weaken the available-bytes guard. + return static_cast(size_) - static_cast(curLen_); + } }; class MemoryOutputStream final : public OutputStream { diff --git a/lang/c++/impl/parsing/ResolvingDecoder.cc b/lang/c++/impl/parsing/ResolvingDecoder.cc index 1553b8a4b62..dd5e342a2c1 100644 --- a/lang/c++/impl/parsing/ResolvingDecoder.cc +++ b/lang/c++/impl/parsing/ResolvingDecoder.cc @@ -471,6 +471,10 @@ class ResolvingDecoderImpl : public ResolvingDecoder { base_->drain(); } + int64_t bytesRemaining() const final { + return base_->bytesRemaining(); + } + public: ResolvingDecoderImpl(const ValidSchema &writer, const ValidSchema &reader, DecoderPtr base) : base_(std::move(base)), diff --git a/lang/c++/impl/parsing/ValidatingCodec.cc b/lang/c++/impl/parsing/ValidatingCodec.cc index 9ec1f040600..58bd5fb79f1 100644 --- a/lang/c++/impl/parsing/ValidatingCodec.cc +++ b/lang/c++/impl/parsing/ValidatingCodec.cc @@ -187,6 +187,10 @@ class ValidatingDecoder : public Decoder { base->drain(); } + int64_t bytesRemaining() const final { + return base->bytesRemaining(); + } + public: ValidatingDecoder(const ValidSchema &s, const shared_ptr &b) : base(b), parser(ValidatingGrammarGenerator().generate(s), NULL, handler_) {} diff --git a/lang/c++/include/avro/Decoder.hh b/lang/c++/include/avro/Decoder.hh index 77b3f9489f7..5342c1cd5e3 100644 --- a/lang/c++/include/avro/Decoder.hh +++ b/lang/c++/include/avro/Decoder.hh @@ -169,6 +169,14 @@ public: /// by the avro decoder. Similar set of problems occur if the Decoder /// consumes more than what it should. virtual void drain() = 0; + + /// Returns the number of bytes that remain to be read from the underlying + /// stream, or a negative value when that count is not known (for example a + /// streaming source, or a decoder not backed by a byte stream). The default + /// is "unknown"; byte-stream decoders override it so a length prefix or a + /// collection block count that exceeds the data actually available can be + /// rejected before allocating for it. + virtual int64_t bytesRemaining() const { return -1; } }; /** diff --git a/lang/c++/include/avro/GenericDatum.hh b/lang/c++/include/avro/GenericDatum.hh index 1b1c3d9af87..fc958f1b56c 100644 --- a/lang/c++/include/avro/GenericDatum.hh +++ b/lang/c++/include/avro/GenericDatum.hh @@ -244,6 +244,9 @@ public: * \param branch The index for the selected branch. */ void selectBranch(size_t branch) { + if (branch >= schema()->leaves()) { + throw Exception("Union branch index out of range: must be less than " + std::to_string(schema()->leaves()) + ", but is " + std::to_string(branch)); + } if (curBranch_ != branch) { datum_ = GenericDatum(schema()->leafAt(branch)); curBranch_ = branch; diff --git a/lang/c++/include/avro/Stream.hh b/lang/c++/include/avro/Stream.hh index de213404d3f..0dd63ee3da0 100644 --- a/lang/c++/include/avro/Stream.hh +++ b/lang/c++/include/avro/Stream.hh @@ -74,6 +74,17 @@ public: * to be used unless, returned back using backup. */ virtual size_t byteCount() const = 0; + + /** + * Returns the number of bytes still available to be read from this + * stream, or a negative value when that count is not known (for example a + * streaming source that cannot report its size). The default is + * "unknown". Memory-backed streams override this so a length prefix that + * exceeds the data actually available can be rejected before allocating + * for it, guarding against an out-of-memory attack from a malicious or + * truncated input. + */ + virtual int64_t remainingBytes() const { return -1; } }; typedef std::unique_ptr InputStreamPtr; @@ -345,6 +356,29 @@ struct StreamReader { return next_ != end_ || fill(); } + /** + * Returns the number of bytes still available to be read: those already + * buffered in this reader plus whatever the underlying stream reports as + * remaining. Returns a negative value when the underlying stream cannot + * report its remaining size. + */ + int64_t remainingBytes() const { + if (in_ == nullptr) { + return -1; + } + int64_t streamRemaining = in_->remainingBytes(); + if (streamRemaining < 0) { + return -1; + } + // Bytes already buffered in this reader, added to what the underlying + // stream still has. Either pointer can be null right after init()/reset() + // (or a partial fill); subtracting when either operand is null is + // undefined behavior, so treat any null pointer as zero buffered and only + // subtract when both point into a real buffer. + int64_t buffered = (next_ == nullptr || end_ == nullptr) ? 0 : (end_ - next_); + return buffered + streamRemaining; + } + /** * Returns unused bytes back to the underlying stream. * If unRead is true the last byte read is also pushed back. diff --git a/lang/c++/test/AvailableBytesTests.cc b/lang/c++/test/AvailableBytesTests.cc new file mode 100644 index 00000000000..76e68da1ef8 --- /dev/null +++ b/lang/c++/test/AvailableBytesTests.cc @@ -0,0 +1,361 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// A bytes/string value is encoded as a length prefix followed by that many +// bytes of data. A malicious or truncated input can declare a huge length with +// little or no actual data, which would otherwise trigger a correspondingly +// huge allocation before the shortfall is noticed. When the input stream can +// report how many bytes remain (a memory-backed stream), the declared length +// must be rejected before allocating for it. + +#include +#include +#include +#include +#include + +#include + +#include "Compiler.hh" +#include "Decoder.hh" +#include "Encoder.hh" +#include "Exception.hh" +#include "Generic.hh" +#include "Stream.hh" +#include "ValidSchema.hh" + +namespace avro { + +// Portable set/unset of the collection-limit environment variable that restores +// any previous value: setenv/unsetenv are POSIX-only (MSVC uses _putenv_s, where +// an empty value unsets), and unconditionally clearing the variable would leak +// state if it was already set before the test ran. +namespace { +const char *const kCollectionItemsEnv = "AVRO_MAX_COLLECTION_ITEMS"; + +class ScopedCollectionLimit { + std::string previous_; + bool hadPrevious_; + + static void setEnv(const char *value) { +#ifdef _WIN32 + _putenv_s(kCollectionItemsEnv, value); +#else + setenv(kCollectionItemsEnv, value, 1); +#endif + } + + static void unsetEnv() { +#ifdef _WIN32 + _putenv_s(kCollectionItemsEnv, ""); +#else + unsetenv(kCollectionItemsEnv); +#endif + } + +public: + explicit ScopedCollectionLimit(const char *value) { + const char *prev = std::getenv(kCollectionItemsEnv); + hadPrevious_ = prev != nullptr; + if (hadPrevious_) { + previous_ = prev; + } + setEnv(value); + } + + ~ScopedCollectionLimit() { + if (hadPrevious_) { + setEnv(previous_.c_str()); + } else { + unsetEnv(); + } + } +}; +} // namespace + +// Reading a bytes value whose declared length far exceeds the tiny backing +// buffer must throw instead of attempting a huge allocation. +static void testDecodeBytesRejectsOversizedLength() { + // 0xFE 0x01 is the zig-zag long 127; the buffer carries no data after it. + const uint8_t data[] = {0xFE, 0x01}; + InputStreamPtr in = memoryInputStream(data, sizeof(data)); + DecoderPtr d = binaryDecoder(); + d->init(*in); + std::vector value; + BOOST_CHECK_THROW(d->decodeBytes(value), Exception); +} + +static void testDecodeStringRejectsOversizedLength() { + const uint8_t data[] = {0xFE, 0x01}; + InputStreamPtr in = memoryInputStream(data, sizeof(data)); + DecoderPtr d = binaryDecoder(); + d->init(*in); + std::string value; + BOOST_CHECK_THROW(d->decodeString(value), Exception); +} + +// A well-formed value whose declared length fits the buffer still decodes. +static void testDecodeStringWithinLimitStillReads() { + // length 3 (zig-zag 6 -> 0x06) followed by "abc". + const uint8_t data[] = {0x06, 'a', 'b', 'c'}; + InputStreamPtr in = memoryInputStream(data, sizeof(data)); + DecoderPtr d = binaryDecoder(); + d->init(*in); + std::string value; + d->decodeString(value); + BOOST_CHECK_EQUAL(value, std::string("abc")); +} + +static void testDecodeBytesWithinLimitStillReads() { + const uint8_t data[] = {0x06, 0x01, 0x02, 0x03}; + InputStreamPtr in = memoryInputStream(data, sizeof(data)); + DecoderPtr d = binaryDecoder(); + d->init(*in); + std::vector value; + d->decodeBytes(value); + BOOST_CHECK_EQUAL(value.size(), 3u); + BOOST_CHECK_EQUAL(value[0], 1); + BOOST_CHECK_EQUAL(value[2], 3); +} + +// An array/map block declares an element count; a malicious or truncated input +// can declare far more elements than the remaining bytes could hold. The count +// is validated against the bytes remaining before resizing, using the minimum +// on-wire size of the element schema (so 0-byte elements like null are not +// falsely rejected). +static GenericDatum decodeCollectionHeader(const ValidSchema &s, int64_t blockCount, + bool addEndMarker) { + // Keep the output stream alive for the whole decode: memoryInputStream + // references the output stream's buffer rather than copying it. + std::unique_ptr os = memoryOutputStream(); + EncoderPtr e = binaryEncoder(); + e->init(*os); + e->encodeLong(blockCount); + if (addEndMarker) { + e->encodeLong(0); + } + e->flush(); + + InputStreamPtr in = memoryInputStream(*os); + DecoderPtr d = binaryDecoder(); + d->init(*in); + GenericDatum datum; + GenericReader::read(*d, datum, s); + return datum; +} + +static void testReadArrayRejectsOversizedCount() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"array\",\"items\":\"long\"}"); + // 1,000,000 long elements declared, but no element data follows. + BOOST_CHECK_THROW(decodeCollectionHeader(s, 1000000, false), Exception); +} + +static void testReadMapRejectsOversizedCount() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"map\",\"values\":\"long\"}"); + BOOST_CHECK_THROW(decodeCollectionHeader(s, 1000000, false), Exception); +} + +static void testReadArrayOfNullNotFalselyRejected() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"array\",\"items\":\"null\"}"); + // 100,000 nulls (zero bytes each) is legitimate and must decode. + GenericDatum datum = decodeCollectionHeader(s, 100000, true); + BOOST_CHECK_EQUAL(datum.value().value().size(), 100000u); +} + +// A deeply but acyclically nested record whose only leaf is null encodes to +// zero bytes, so the per-element minimum must be 0 and a large array of such +// records must not be falsely rejected. This exercises the recursion depth +// guard in minBytesPerElement(), which must yield a conservative lower bound +// (0) rather than over-estimating for a legitimately deep schema. +static void testReadArrayOfDeeplyNestedNullNotFalselyRejected() { + // Build ~70 nested records: R0 { null f; }, R1 { R0 f; }, ... The nesting + // exceeds the depth guard, yet every leaf is null so the true minimum is 0. + std::string schema = "\"null\""; + for (int i = 0; i < 70; ++i) { + schema = "{\"type\":\"record\",\"name\":\"R" + std::to_string(i) + + "\",\"fields\":[{\"name\":\"f\",\"type\":" + schema + "}]}"; + } + ValidSchema s = compileJsonSchemaFromString( + ("{\"type\":\"array\",\"items\":" + schema + "}").c_str()); + // 100,000 zero-byte elements is legitimate and must decode. + GenericDatum datum = decodeCollectionHeader(s, 100000, true); + BOOST_CHECK_EQUAL(datum.value().value().size(), 100000u); +} + +// Calling bytesRemaining() immediately after init(), before any data has been +// buffered, must be well-defined (the underlying StreamReader must not subtract +// null buffer pointers) and report the whole stream as available. +static void testBytesRemainingRightAfterInit() { + const uint8_t data[] = {0x06, 'a', 'b', 'c'}; + InputStreamPtr in = memoryInputStream(data, sizeof(data)); + DecoderPtr d = binaryDecoder(); + d->init(*in); + BOOST_CHECK_EQUAL(d->bytesRemaining(), static_cast(sizeof(data))); +} + +// Zero-byte elements (null, a record with only zero-byte fields) consume no +// input, so ensureCollectionAvailable cannot bound their count. A huge declared +// block count of such elements is capped against a configurable limit. +static GenericDatum decodeLongs(const ValidSchema &s, const std::vector &longs) { + std::unique_ptr os = memoryOutputStream(); + EncoderPtr e = binaryEncoder(); + e->init(*os); + for (int64_t v : longs) { + e->encodeLong(v); + } + e->flush(); + InputStreamPtr in = memoryInputStream(*os); + DecoderPtr d = binaryDecoder(); + d->init(*in); + GenericDatum datum; + GenericReader::read(*d, datum, s); + return datum; +} + +static void testReadArrayOfNullRejectsCountAboveDefaultLimit() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"array\",\"items\":\"null\"}"); + // The reported exploit: 200,000,000 nulls rejected by the default limit. + BOOST_CHECK_THROW(decodeCollectionHeader(s, 200000000, true), Exception); +} + +static void testReadArrayOfAllNullRecordRejectsHugeCount() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"array\",\"items\":" + "{\"type\":\"record\",\"name\":\"R\",\"fields\":[{\"name\":\"n\",\"type\":\"null\"}]}}"); + BOOST_CHECK_THROW(decodeCollectionHeader(s, 200000000, true), Exception); +} + +static void testReadArrayOfNullRejectsNegativeCount() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"array\",\"items\":\"null\"}"); + // Negative count (-200M), block byte-size 0, end marker 0: normalized to + // 200M and still bounded. + BOOST_CHECK_THROW(decodeLongs(s, {-200000000, 0, 0}), Exception); +} + +static void testReadMapOfNullRejectedByAvailableBytes() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"map\",\"values\":\"null\"}"); + // Each map entry carries a >= 1 byte key, so a huge map is bounded by + // the bytes-remaining check. + BOOST_CHECK_THROW(decodeCollectionHeader(s, 200000000, false), Exception); +} + +static void testReadArrayOfNullRespectsConfiguredLimit() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"array\",\"items\":\"null\"}"); + ScopedCollectionLimit limitGuard("1000"); + // Within the limit decodes; over the limit and cumulative are rejected. + GenericDatum ok = decodeCollectionHeader(s, 1000, true); + BOOST_CHECK_EQUAL(ok.value().value().size(), 1000u); + BOOST_CHECK_THROW(decodeCollectionHeader(s, 1001, true), Exception); + BOOST_CHECK_THROW(decodeLongs(s, {600, 600, 0}), Exception); +} + +// A backed non-zero-byte array that passes the bytes check is still bounded by +// the structural cap (exercised with a lowered limit). +static void testReadArrayOfLongRejectedByStructuralCap() { + ValidSchema s = compileJsonSchemaFromString( + "{\"type\":\"array\",\"items\":\"long\"}"); + ScopedCollectionLimit limitGuard("5"); + // 10 real longs: block count 10, ten 1-byte longs, end marker. + std::vector longs = {10, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0}; + BOOST_CHECK_THROW(decodeLongs(s, longs), Exception); +} + +// Skipping a huge array (via the decoder's skipArray) is bounded by the +// structural cap. +static void testSkipArrayRejectsHugeCount() { + ScopedCollectionLimit limitGuard("1000"); + std::unique_ptr os = memoryOutputStream(); + EncoderPtr e = binaryEncoder(); + e->init(*os); + e->encodeLong(2000); // block count 2000, no data needed (skip throws first) + e->flush(); + InputStreamPtr in = memoryInputStream(*os); + DecoderPtr d = binaryDecoder(); + d->init(*in); + BOOST_CHECK_THROW(d->skipArray(), Exception); +} + +static void testSkipArrayRejectsNegativeBlockSize() { + // A negative block count is followed by a block byte-size; a negative size + // would convert to a huge size_t and drive an unbounded skip, so it must be + // rejected. + std::unique_ptr os = memoryOutputStream(); + EncoderPtr e = binaryEncoder(); + e->init(*os); + e->encodeLong(-3); // negative block count: a byte-size follows + e->encodeLong(-1); // negative block byte-size + e->flush(); + InputStreamPtr in = memoryInputStream(*os); + DecoderPtr d = binaryDecoder(); + d->init(*in); + BOOST_CHECK_THROW(d->skipArray(), Exception); +} + +// A union branch index outside [0, branch count) is malformed and must be +// rejected with an Avro Exception rather than letting leafAt() throw +// std::out_of_range. +static void testReadUnionRejectsOutOfRangeIndex() { + ValidSchema s = compileJsonSchemaFromString("[\"null\",\"long\"]"); + for (int64_t index : {int64_t(5), int64_t(-1)}) { + std::unique_ptr os = memoryOutputStream(); + EncoderPtr e = binaryEncoder(); + e->init(*os); + e->encodeLong(index); // raw branch index; only 2 branches exist + e->flush(); + InputStreamPtr in = memoryInputStream(*os); + DecoderPtr d = binaryDecoder(); + d->init(*in); + GenericDatum datum; + BOOST_CHECK_THROW(GenericReader::read(*d, datum, s), Exception); + } +} + +} // namespace avro + +boost::unit_test::test_suite * +init_unit_test_suite(int, char *[]) { + using namespace boost::unit_test; + + auto *ts = BOOST_TEST_SUITE("Avro C++ available-bytes tests"); + ts->add(BOOST_TEST_CASE(&avro::testDecodeBytesRejectsOversizedLength)); + ts->add(BOOST_TEST_CASE(&avro::testDecodeStringRejectsOversizedLength)); + ts->add(BOOST_TEST_CASE(&avro::testDecodeStringWithinLimitStillReads)); + ts->add(BOOST_TEST_CASE(&avro::testDecodeBytesWithinLimitStillReads)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayRejectsOversizedCount)); + ts->add(BOOST_TEST_CASE(&avro::testReadMapRejectsOversizedCount)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayOfNullNotFalselyRejected)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayOfDeeplyNestedNullNotFalselyRejected)); + ts->add(BOOST_TEST_CASE(&avro::testBytesRemainingRightAfterInit)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayOfNullRejectsCountAboveDefaultLimit)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayOfAllNullRecordRejectsHugeCount)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayOfNullRejectsNegativeCount)); + ts->add(BOOST_TEST_CASE(&avro::testReadMapOfNullRejectedByAvailableBytes)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayOfNullRespectsConfiguredLimit)); + ts->add(BOOST_TEST_CASE(&avro::testReadArrayOfLongRejectedByStructuralCap)); + ts->add(BOOST_TEST_CASE(&avro::testSkipArrayRejectsHugeCount)); + ts->add(BOOST_TEST_CASE(&avro::testSkipArrayRejectsNegativeBlockSize)); + ts->add(BOOST_TEST_CASE(&avro::testReadUnionRejectsOutOfRangeIndex)); + return ts; +} diff --git a/lang/c++/test/CodecTests.cc b/lang/c++/test/CodecTests.cc index 59aa023dbd1..11231151de7 100644 --- a/lang/c++/test/CodecTests.cc +++ b/lang/c++/test/CodecTests.cc @@ -2145,6 +2145,32 @@ static void testByteCount() { BOOST_CHECK_EQUAL(os1->byteCount(), 3); } +static void testArrayInt64MinBlockCount() { + // INT64_MIN encoded as a zigzag varint is the 10-byte sequence + // FF FF FF FF FF FF FF FF FF 01. As an array block count its negation + // overflows int64_t, so the decoder must reject it rather than return 2^63. + const uint8_t data[] = {0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0x01}; + + InputStreamPtr is = memoryInputStream(data, sizeof(data)); + DecoderPtr d = binaryDecoder(); + d->init(*is); + + BOOST_CHECK_THROW(d->arrayStart(), Exception); +} + +static void testMapInt64MinBlockCount() { + // Same INT64_MIN varint, but as a map block count. + const uint8_t data[] = {0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0x01}; + + InputStreamPtr is = memoryInputStream(data, sizeof(data)); + DecoderPtr d = binaryDecoder(); + d->init(*is); + + BOOST_CHECK_THROW(d->mapStart(), Exception); +} + } // namespace avro boost::unit_test::test_suite * @@ -2161,6 +2187,8 @@ init_unit_test_suite(int, char *[]) { ts->add(BOOST_TEST_CASE(avro::testJsonCodecReinit)); ts->add(BOOST_TEST_CASE(avro::testArrayNegativeBlockCount)); ts->add(BOOST_TEST_CASE(avro::testByteCount)); + ts->add(BOOST_TEST_CASE(avro::testArrayInt64MinBlockCount)); + ts->add(BOOST_TEST_CASE(avro::testMapInt64MinBlockCount)); return ts; }