mirror of
https://github.com/ClickHouse/ClickHouse.git
synced 2024-09-20 08:40:50 +00:00
Merge pull request #6247 from yandex/fix-malicious-replica
Fixed the case when malicious ClickHouse replica can force clickhouse-server to write to arbitrary path.
This commit is contained in:
commit
eeeab85bbe
@ -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;
|
||||
|
@ -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
|
||||
@ -200,7 +201,7 @@ MergeTreeData::MutableDataPartPtr Fetcher::fetchPart(
|
||||
String tmp_prefix = tmp_prefix_.empty() ? TMP_PREFIX : tmp_prefix_;
|
||||
|
||||
String relative_part_path = String(to_detached ? "detached/" : "") + tmp_prefix + part_name;
|
||||
String absolute_part_path = data.getFullPath() + relative_part_path + "/";
|
||||
String absolute_part_path = Poco::Path(data.getFullPath() + relative_part_path + "/").absolute().toString();
|
||||
Poco::File part_file(absolute_part_path);
|
||||
|
||||
if (part_file.exists())
|
||||
@ -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 absolute_file_path = Poco::Path(absolute_part_path + file_name).absolute().toString();
|
||||
if (!startsWith(absolute_file_path, absolute_part_path))
|
||||
throw Exception("File path (" + absolute_file_path + ") doesn't appear to be inside part path (" + absolute_part_path + ")."
|
||||
" This may happen if we are trying to download part from malicious replica or logical error.",
|
||||
ErrorCodes::INSECURE_PATH);
|
||||
|
||||
WriteBufferFromFile file_out(absolute_file_path);
|
||||
HashingWriteBuffer hashing_out(file_out);
|
||||
copyData(in, hashing_out, file_size, blocker.getCounter());
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user