diff --git a/clickhouse/columns/lowcardinality.cpp b/clickhouse/columns/lowcardinality.cpp index 22b1995b..59b9e06e 100644 --- a/clickhouse/columns/lowcardinality.cpp +++ b/clickhouse/columns/lowcardinality.cpp @@ -46,7 +46,8 @@ enum IndexFlag { NeedUpdateDictionary = 1u << 10u }; -ColumnRef createIndexColumn(IndexType type) { +ColumnRef createIndexColumn(uint64_t type) { + // Switch on the raw wire value: converting an out-of-range value to IndexType is undefined. switch (type) { case IndexType::UInt8: return std::make_shared(); @@ -58,7 +59,33 @@ ColumnRef createIndexColumn(IndexType type) { return std::make_shared(); } - throw ValidationError("Invalid LowCardinality index type value: " + std::to_string(static_cast(type))); + throw ValidationError("Invalid LowCardinality index type value: " + std::to_string(type)); +} + +template +void validateIndexes(Column& index_column, size_t dictionary_size) { + const auto& indexes = static_cast(index_column).GetWritableData(); + for (size_t i = 0, rows = indexes.size(); i < rows; ++i) { + if (indexes[i] >= dictionary_size) { + throw ProtocolError("LowCardinality index " + std::to_string(indexes[i]) + " at row " + std::to_string(i) + + " is out of range for a dictionary of " + std::to_string(dictionary_size) + " items"); + } + } +} + +void validateIndexes(Column& index_column, size_t dictionary_size) { + switch (index_column.Type()->GetCode()) { + case Type::UInt8: + return validateIndexes(index_column, dictionary_size); + case Type::UInt16: + return validateIndexes(index_column, dictionary_size); + case Type::UInt32: + return validateIndexes(index_column, dictionary_size); + case Type::UInt64: + return validateIndexes(index_column, dictionary_size); + default: + throw ValidationError("Invalid index column type for LowCardinality column:" + index_column.Type()->GetName()); + } } IndexType indexTypeFromIndexColumn(const Column & index_column) { @@ -426,7 +453,7 @@ auto Load(ColumnRef new_dictionary_column, InputStream& input, size_t rows) { if (!WireFormat::ReadFixed(input, &index_serialization_type)) throw ProtocolError("Failed to read index serializaton type."); - auto new_index_column = createIndexColumn(static_cast(index_serialization_type & IndexTypeMask)); + auto new_index_column = createIndexColumn(index_serialization_type & IndexTypeMask); if (index_serialization_type & IndexFlag::NeedGlobalDictionaryBit) throw UnimplementedError("Global dictionary is not supported."); @@ -452,7 +479,12 @@ auto Load(ColumnRef new_dictionary_column, InputStream& input, size_t rows) { if (number_of_rows != rows) throw AssertionError("LowCardinality column must be read in full."); - new_index_column->LoadBody(&input, number_of_rows); + if (!new_index_column->LoadBody(&input, number_of_rows)) { + throw ProtocolError("Failed to read values of index column."); + } + + // GetItem() dereferences the dictionary with these indexes unchecked. + validateIndexes(*new_index_column, dataColumn->Size()); if (auto nullable = new_dictionary_column->As()) { nullable->Append(true); diff --git a/ut/columns_ut.cpp b/ut/columns_ut.cpp index 57650ea9..322a1fbf 100644 --- a/ut/columns_ut.cpp +++ b/ut/columns_ut.cpp @@ -1360,6 +1360,82 @@ TEST(ColumnsCase, ColumnLowCardinalityString_Load) { } } +namespace { +// Native LowCardinality body: key version, index type (0..3 for UInt8..UInt64) with +// HasAdditionalKeys, dictionary keys, row count and the indexes at that width. +std::string MakeLowCardinalityBody(uint64_t index_type, uint64_t keys, const std::string& dictionary, + const std::vector& indexes) { + std::string out; + auto put = [&out](uint64_t v, size_t width) { out.append(reinterpret_cast(&v), width); }; + put(1, 8); + put(index_type | (1u << 9), 8); + put(keys, 8); + out += dictionary; + put(indexes.size(), 8); + for (auto index : indexes) { + put(index, size_t{1} << index_type); + } + return out; +} +} // namespace + +TEST(ColumnsCase, ColumnLowCardinalityFixedString_LoadRejectsIndexOutOfRange) { + // 1001 * index wraps modulo 2^64 to byte 1500 of the 2002-byte dictionary. + const uint64_t wrapping_index = 12107403452974201212ull; + const std::string dictionary = std::string(1001, 'A') + std::string(1001, 'B'); + + for (const auto& [index_type, index] : std::vector>{{3, 2}, {3, wrapping_index}, {0, 2}}) { + ColumnLowCardinalityT col(1001); + const auto body = MakeLowCardinalityBody(index_type, 2, dictionary, {1, index}); + ArrayInput input(body.data(), body.size()); + EXPECT_FALSE(col.Load(&input, 2)) << "index type " << index_type << " index " << index; + } + + for (uint64_t index_type : {0, 3}) { + ColumnLowCardinalityT col(1001); + const auto body = MakeLowCardinalityBody(index_type, 2, dictionary, {1, 0}); + ArrayInput input(body.data(), body.size()); + ASSERT_TRUE(col.Load(&input, 2)) << "index type " << index_type; + EXPECT_EQ(col.At(0), std::string(1001, 'B')); + EXPECT_EQ(col.At(1), std::string(1001, 'A')); + } +} + +TEST(ColumnsCase, ColumnLowCardinalityString_LoadRejectsIndexOutOfRange) { + const std::string dictionary = std::string{0, 3} + "foo"; + auto make_nullable = [] { + return ColumnLowCardinality(std::make_shared(std::make_shared(), std::make_shared())); + }; + { + ColumnLowCardinalityT col; + const auto body = MakeLowCardinalityBody(3, 2, dictionary, {1, 2}); + ArrayInput input(body.data(), body.size()); + EXPECT_FALSE(col.Load(&input, 2)); + } + { + auto col = make_nullable(); + const auto body = MakeLowCardinalityBody(3, 2, dictionary, {0, 2}); + ArrayInput input(body.data(), body.size()); + EXPECT_FALSE(col.Load(&input, 2)); + } + { + ColumnLowCardinalityT col; + const auto body = MakeLowCardinalityBody(3, 2, dictionary, {1, 0}); + ArrayInput input(body.data(), body.size()); + ASSERT_TRUE(col.Load(&input, 2)); + EXPECT_EQ(col.At(0), "foo"); + EXPECT_EQ(col.At(1), ""); + } + + // Index 0 of a Nullable dictionary is the null item. + auto col = make_nullable(); + const auto body = MakeLowCardinalityBody(0, 2, dictionary, {0, 1}); + ArrayInput input(body.data(), body.size()); + ASSERT_TRUE(col.Load(&input, 2)); + EXPECT_EQ(col.GetItem(0).type, Type::Void); + EXPECT_EQ(col.GetItem(1).AsBinaryData(), "foo"); +} + // This is temporary disabled since we are not 100% compatitable with ClickHouse // on how we serailize LC columns, but we check interoperability in other tests (see client_ut.cpp) TEST(ColumnsCase, DISABLED_ColumnLowCardinalityString_Save) {