From d559d5b478389e0062d6dc7c4df0b5a2097043d9 Mon Sep 17 00:00:00 2001 From: mohammed arib Date: Tue, 21 Jul 2026 12:49:52 +0530 Subject: [PATCH] validate union branch and enum index in the resolving reader The branch index and enum ordinal come straight off the wire and were used to index the resolver vectors unchecked; the enum bound was an assert that Release builds drop. Reject out-of-range values instead. --- lang/c++/impl/Resolver.cc | 17 ++++++++++- lang/c++/test/unittest.cc | 61 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/lang/c++/impl/Resolver.cc b/lang/c++/impl/Resolver.cc index b7a57197bec..95b3aeddaeb 100644 --- a/lang/c++/impl/Resolver.cc +++ b/lang/c++/impl/Resolver.cc @@ -46,6 +46,15 @@ NoOp noop; #define DEBUG_OUT(str) noop << str #endif +// The branch index is read straight off the wire, so it has to be checked +// against the writer union before it is used to index the resolver vectors. +static void checkUnionChoice(size_t choice, size_t branches) { + if (choice >= branches) { + throw Exception("Union selection out of range, got {}, expecting 0-{}", + choice, branches - 1); + } +} + template class PrimitiveSkipper : public Resolver { public: @@ -307,7 +316,10 @@ class EnumParser : public Resolver { void parse(Reader &reader, uint8_t *address) const final { auto val = static_cast(reader.readEnum()); - assert(val < mapping_.size()); + if (val >= mapping_.size()) { + throw Exception("Enum index out of range, got {}, expecting 0-{}", + val, mapping_.size() - 1); + } if (mapping_[val] < readerSize_) { auto *location = reinterpret_cast(address + offset_); @@ -329,6 +341,7 @@ class UnionSkipper : public Resolver { void parse(Reader &reader, uint8_t *address) const final { DEBUG_OUT("Skipping union"); auto choice = static_cast(reader.readUnion()); + checkUnionChoice(choice, resolvers_.size()); resolvers_[choice]->parse(reader, address); } @@ -345,6 +358,7 @@ class UnionParser : public Resolver { void parse(Reader &reader, uint8_t *address) const final { DEBUG_OUT("Reading union"); auto writerChoice = static_cast(reader.readUnion()); + checkUnionChoice(writerChoice, resolvers_.size()); auto *readerChoice = reinterpret_cast(address + choiceOffset_); *readerChoice = choiceMapping_[writerChoice]; @@ -375,6 +389,7 @@ class UnionToNonUnionParser : public Resolver { void parse(Reader &reader, uint8_t *address) const final { DEBUG_OUT("Reading union to non-union"); auto choice = static_cast(reader.readUnion()); + checkUnionChoice(choice, resolvers_.size()); resolvers_[choice]->parse(reader, address); } diff --git a/lang/c++/test/unittest.cc b/lang/c++/test/unittest.cc index b0cb44c5b9f..7fe35a773be 100644 --- a/lang/c++/test/unittest.cc +++ b/lang/c++/test/unittest.cc @@ -24,8 +24,11 @@ #include "Compiler.hh" #include "Decoder.hh" #include "Encoder.hh" +#include "Layout.hh" #include "Node.hh" #include "Parser.hh" +#include "ResolverSchema.hh" +#include "ResolvingReader.hh" #include "Schema.hh" #include "SchemaResolution.hh" #include "Serializer.hh" @@ -912,10 +915,68 @@ struct TestBadStuff { std::cout << "(intentional) error: " << error << '\n'; } + // A union branch index and an enum ordinal are read straight off the wire, + // so a corrupt or hostile datum can name a branch the writer schema does + // not have. Both must be rejected instead of indexing out of range. + void testOutOfRangeUnionIndex() { + std::cout << "TestOutOfRangeUnionIndex\n"; + + avro::ValidSchema writer = avro::compileJsonSchemaFromString( + R"({"type":"record","name":"R","fields":[)" + R"({"name":"x","type":["null","int"]}]})"); + avro::ValidSchema reader = avro::compileJsonSchemaFromString( + R"({"type":"record","name":"R","fields":[)" + R"({"name":"y","type":"int"}]})"); + + avro::CompoundLayout layout(0); + std::unique_ptr field(new avro::PrimitiveLayout(0)); + layout.add(field); + + // zigzag(10000) == branch 5000, the writer union has 2 branches + const char data[] = {'\x90', '\x4e'}; + avro::OutputBuffer buf; + buf.writeTo(data, sizeof(data)); + + avro::ResolverSchema xSchema(writer, reader, layout); + avro::ResolvingReader r(xSchema, avro::InputBuffer(buf)); + + uint8_t object[64] = {0}; + BOOST_CHECK_THROW(r.parse(object), avro::Exception); + } + + void testOutOfRangeEnumIndex() { + std::cout << "TestOutOfRangeEnumIndex\n"; + + const char *json = + R"({"type":"record","name":"R","fields":[{"name":"e","type":)" + R"({"type":"enum","name":"E","symbols":["A","B"]}}]})"; + avro::ValidSchema schema = avro::compileJsonSchemaFromString(json); + + avro::CompoundLayout layout(0); + auto *enumLayout = new avro::CompoundLayout(0); + std::unique_ptr value(new avro::PrimitiveLayout(0)); + enumLayout->add(value); + std::unique_ptr field(enumLayout); + layout.add(field); + + // zigzag(10000) == ordinal 5000, the enum has 2 symbols + const char data[] = {'\x90', '\x4e'}; + avro::OutputBuffer buf; + buf.writeTo(data, sizeof(data)); + + avro::ResolverSchema xSchema(schema, schema, layout); + avro::ResolvingReader r(xSchema, avro::InputBuffer(buf)); + + uint8_t object[64] = {0}; + BOOST_CHECK_THROW(r.parse(object), avro::Exception); + } + void test() { std::cout << "TestBadStuff\n"; testBadFile(); testBadSchema(); + testOutOfRangeUnionIndex(); + testOutOfRangeEnumIndex(); } };