From bc9d620155fca38b2a495fe3080ef085e9179f41 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 31 Jul 2019 21:21:13 +0300 Subject: [PATCH] Fixed the case when malicious ClickHouse replica can force clickhouse-server to write to arbitrary path --- dbms/src/Common/ErrorCodes.cpp | 1 + dbms/src/Storages/MergeTree/DataPartsExchange.cpp | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dbms/src/Common/ErrorCodes.cpp b/dbms/src/Common/ErrorCodes.cpp index 5cb6b7e0a3..bb1636b787 100644 --- a/dbms/src/Common/ErrorCodes.cpp +++ b/dbms/src/Common/ErrorCodes.cpp @@ -440,6 +440,7 @@ namespace ErrorCodes extern const int CANNOT_FCNTL = 463; extern const int CANNOT_PARSE_ELF = 464; extern const int CANNOT_PARSE_DWARF = 465; + extern const int INSECURE_PATH = 466; extern const int KEEPER_EXCEPTION = 999; extern const int POCO_EXCEPTION = 1000; diff --git a/dbms/src/Storages/MergeTree/DataPartsExchange.cpp b/dbms/src/Storages/MergeTree/DataPartsExchange.cpp index 42c668c9fc..ee7de07077 100644 --- a/dbms/src/Storages/MergeTree/DataPartsExchange.cpp +++ b/dbms/src/Storages/MergeTree/DataPartsExchange.cpp @@ -27,6 +27,7 @@ namespace ErrorCodes extern const int CANNOT_WRITE_TO_OSTREAM; extern const int CHECKSUM_DOESNT_MATCH; extern const int UNKNOWN_TABLE; + extern const int INSECURE_PATH; } namespace DataPartsExchange @@ -225,7 +226,15 @@ MergeTreeData::MutableDataPartPtr Fetcher::fetchPart( readStringBinary(file_name, in); readBinary(file_size, in); - WriteBufferFromFile file_out(absolute_part_path + file_name); + /// File must be inside "absolute_part_path" directory. + /// Otherwise malicious ClickHouse replica may force us to write to arbitrary path. + String file_absolute_path = Poco::Path(absolute_part_path + file_name).absolute().toString(); + if (!startsWith(file_absolute_path, absolute_part_path)) + throw Exception("File path doesn't appear to be inside part path." + " This may happen if we are trying to download part from malicious replica or logical error.", + ErrorCodes::INSECURE_PATH); + + WriteBufferFromFile file_out(file_absolute_path); HashingWriteBuffer hashing_out(file_out); copyData(in, hashing_out, file_size, blocker.getCounter()); -- GitLab