From c78b94bed6100b4e09007d6ad8aad547712be84d Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Fri, 18 Aug 2023 17:55:28 +0200 Subject: [PATCH 1/2] Fix possible UB in inverted indexes (experimental feature) It is possible to have incorrect types there in case of index contains functions, add a proper check after value had been changed.
MSan report ``` ==182==WARNING: MemorySanitizer: use-of-uninitialized-value 0 0x55658547c59e in DB::MergeTreeConditionInverted::traverseASTEquals() build_docker/./src/Storages/MergeTree/MergeTreeIndexInverted.cpp:551:26 1 0x556585475566 in DB::MergeTreeConditionInverted::traverseAtomAST() build_docker/./src/Storages/MergeTree/MergeTreeIndexInverted.cpp:438:21 2 0x55658547e73e in DB::MergeTreeConditionInverted::MergeTreeConditionInverted()::$_1::operator()(DB::RPNBuilderTreeNode const&, DB::MergeTreeConditionInverted::RPNElement&) const build_docker/./src/Storages/MergeTree/MergeTreeIndexInverted.cpp:228:73 9 0x556585489e35 in DB::RPNBuilder::traverseTree(DB::RPNBuilderTreeNode const&) build_docker/./src/Storages/MergeTree/RPNBuilder.h:252:14 10 0x556585489921 in DB::RPNBuilder::traverseTree(DB::RPNBuilderTreeNode const&) build_docker/./src/Storages/MergeTree/RPNBuilder.h:239:21 11 0x556585489921 in DB::RPNBuilder::traverseTree(DB::RPNBuilderTreeNode const&) build_docker/./src/Storages/MergeTree/RPNBuilder.h:239:21 12 0x5565854804e6 in DB::RPNBuilder::RPNBuilder(std::__1::shared_ptr const&, std::__1::shared_ptr, DB::Block, std::__1::shared_ptr, std::__1::function const&) build_docker/./src/Storages/MergeTree/RPNBuilder.h:218:9 13 0x55658546fb87 in DB::MergeTreeConditionInverted::MergeTreeConditionInverted(DB::SelectQueryInfo const&, std::__1::shared_ptr, DB::Block const&, DB::GinFilterParameters const&, DB::ITokenExtractor const*) build_docker/./src/Storages/MergeTree/MergeTreeIndexInverted.cpp:223:28 19 0x55658547ceda in DB::MergeTreeIndexInverted::createIndexCondition(DB::SelectQueryInfo const&, std::__1::shared_ptr) const build_docker/./src/Storages/MergeTree/MergeTreeIndexInverted.cpp:716:12 20 0x556587125734 in DB::buildIndexes(std::__1::optional&, std::__1::shared_ptr, DB::MergeTreeData const&, std::__1::shared_ptr const&, DB::SelectQueryInfo const&, std::__1::shared_ptr const&) build_docker/./src/Processors/QueryPlan/ReadFromMergeTree.cpp:1292:48 21 0x556587121aaa in DB::ReadFromMergeTree::applyFilters() build_docker/./src/Processors/QueryPlan/ReadFromMergeTree.cpp:1305:5 22 0x55658726cf4f in DB::QueryPlanOptimizations::optimizeTreeThirdPass(DB::QueryPlan::Node&, std::__1::list>&) build_docker/./src/Processors/QueryPlan/Optimizations/optimizeTree.cpp:201:38 23 0x5565870bc489 in DB::QueryPlan::optimize(DB::QueryPlanOptimizationSettings const&) build_docker/./src/Processors/QueryPlan/QueryPlan.cpp:485:5 24 0x5565870b919a in DB::QueryPlan::buildQueryPipeline(DB::QueryPlanOptimizationSettings const&, DB::BuildQueryPipelineSettings const&) build_docker/./src/Processors/QueryPlan/QueryPlan.cpp:161:5 25 0x55658206385a in DB::InterpreterSelectWithUnionQuery::execute() build_docker/./src/Interpreters/InterpreterSelectWithUnionQuery.cpp:379:31 26 0x556582c15c50 in DB::executeQueryImpl(char const*, char const*, std::__1::shared_ptr, bool, DB::QueryProcessingStage::Enum, DB::ReadBuffer*) build_docker/./src/Interpreters/executeQuery.cpp:751:40 27 0x556582c09681 in DB::executeQuery(std::__1::basic_string, std::__1::allocator> const&, std::__1::shared_ptr, bool, DB::QueryProcessingStage::Enum) build_docker/./src/Interpreters/executeQuery.cpp:1173:30 28 0x5565860e7bd7 in DB::TCPHandler::runImpl() build_docker/./src/Server/TCPHandler.cpp:421:24 29 0x556586126e9e in DB::TCPHandler::run() build_docker/./src/Server/TCPHandler.cpp:2057:9 ```
Signed-off-by: Azat Khuzhin --- src/Storages/MergeTree/MergeTreeIndexInverted.cpp | 4 ++++ .../02862_index_inverted_incorrect_args.reference | 0 .../0_stateless/02862_index_inverted_incorrect_args.sql | 8 ++++++++ 3 files changed, 12 insertions(+) create mode 100644 tests/queries/0_stateless/02862_index_inverted_incorrect_args.reference create mode 100644 tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql diff --git a/src/Storages/MergeTree/MergeTreeIndexInverted.cpp b/src/Storages/MergeTree/MergeTreeIndexInverted.cpp index 6b4919c545d..325df6ffb6f 100644 --- a/src/Storages/MergeTree/MergeTreeIndexInverted.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexInverted.cpp @@ -491,6 +491,10 @@ bool MergeTreeConditionInverted::traverseASTEquals( DataTypePtr const_type; if (argument.tryGetConstant(const_value, const_type)) { + auto const_data_type = WhichDataType(const_type); + if (!const_data_type.isStringOrFixedString() && !const_data_type.isArray()) + return false; + key_column_num = header.getPositionByName(map_keys_index_column_name); key_exists = true; } diff --git a/tests/queries/0_stateless/02862_index_inverted_incorrect_args.reference b/tests/queries/0_stateless/02862_index_inverted_incorrect_args.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql b/tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql new file mode 100644 index 00000000000..e3d9adc1574 --- /dev/null +++ b/tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql @@ -0,0 +1,8 @@ +-- https://github.com/ClickHouse/ClickHouse/issues/52019 +DROP TABLE IF EXISTS tab; +SET allow_experimental_inverted_index=1; +CREATE TABLE tab (`k` UInt64, `s` Map(String, String), INDEX af mapKeys(s) TYPE inverted(2) GRANULARITY 1) ENGINE = MergeTree ORDER BY k SETTINGS index_granularity = 2, index_granularity_bytes = '10Mi'; +INSERT INTO tab (k) VALUES (0); +SELECT * FROM tab PREWHERE (s[NULL]) = 'Click a03' SETTINGS allow_experimental_analyzer=1; -- { serverError ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER } +SELECT * FROM tab PREWHERE (s[1]) = 'Click a03' SETTINGS allow_experimental_analyzer=1; -- { serverError ILLEGAL_TYPE_OF_ARGUMENT } +SELECT * FROM tab PREWHERE (s['foo']) = 'Click a03' SETTINGS allow_experimental_analyzer=1; From 1e4f548cbcd78e37d33917c2e7dff25d5c366a11 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 18 Aug 2023 21:46:54 +0300 Subject: [PATCH 2/2] Update 02862_index_inverted_incorrect_args.sql --- .../queries/0_stateless/02862_index_inverted_incorrect_args.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql b/tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql index e3d9adc1574..0678023f2f4 100644 --- a/tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql +++ b/tests/queries/0_stateless/02862_index_inverted_incorrect_args.sql @@ -6,3 +6,4 @@ INSERT INTO tab (k) VALUES (0); SELECT * FROM tab PREWHERE (s[NULL]) = 'Click a03' SETTINGS allow_experimental_analyzer=1; -- { serverError ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER } SELECT * FROM tab PREWHERE (s[1]) = 'Click a03' SETTINGS allow_experimental_analyzer=1; -- { serverError ILLEGAL_TYPE_OF_ARGUMENT } SELECT * FROM tab PREWHERE (s['foo']) = 'Click a03' SETTINGS allow_experimental_analyzer=1; +DROP TABLE tab;