Merge pull request #41709 from ClickHouse/backport/22.9/41694

Backport #41694 to 22.9: Check file path for path traversal attacks in errors logger for input formats
ClickHouse · Sep 23, 2022 · 362e2ce · 362e2ce
2 parents 6517ea2 + d2161f9
commit 362e2ce
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 4 deletions.
diff --git a/src/Processors/Formats/InputFormatErrorsLogger.cpp b/src/Processors/Formats/InputFormatErrorsLogger.cpp
@@ -1,15 +1,20 @@
 #include <Processors/Formats/InputFormatErrorsLogger.h>
+#include <Processors/Formats/IRowOutputFormat.h>
 #include <DataTypes/DataTypeDateTime.h>
 #include <DataTypes/DataTypeNullable.h>
 #include <DataTypes/DataTypeString.h>
 #include <DataTypes/DataTypesNumber.h>
-#include <IO/WriteHelpers.h>
-#include <Processors/Formats/IRowOutputFormat.h>
+#include <Common/filesystemHelpers.h>
 
 
 namespace DB
 {
 
+namespace ErrorCodes
+{
+    extern const int DATABASE_ACCESS_DENIED;
+}
+
 namespace
 {
     const String DEFAULT_OUTPUT_FORMAT = "CSV";
@@ -26,8 +31,19 @@ InputFormatErrorsLogger::InputFormatErrorsLogger(const ContextPtr & context)
         database = context->getInsertionTable().getDatabaseName();
 
     String path_in_setting = context->getSettingsRef().input_format_record_errors_file_path;
-    errors_file_path = context->getApplicationType() == Context::ApplicationType::SERVER ? context->getUserFilesPath() + path_in_setting
-                                                                                         : path_in_setting;
+
+    if (context->getApplicationType() == Context::ApplicationType::SERVER)
+    {
+        auto user_files_path = context->getUserFilesPath();
+        errors_file_path = fs::path(user_files_path) / path_in_setting;
+        if (!fileOrSymlinkPathStartsWith(errors_file_path, user_files_path))
+            throw Exception(ErrorCodes::DATABASE_ACCESS_DENIED, "Cannot log errors in path `{}`, because it is not inside `{}`", errors_file_path, user_files_path);
+    }
+    else
+    {
+        errors_file_path = path_in_setting;
+    }
+
     while (fs::exists(errors_file_path))
     {
         errors_file_path += "_new";

diff --git a/tests/queries/0_stateless/02453_check_path_in_errors_logger.reference b/tests/queries/0_stateless/02453_check_path_in_errors_logger.reference
diff --git a/tests/queries/0_stateless/02453_check_path_in_errors_logger.sql b/tests/queries/0_stateless/02453_check_path_in_errors_logger.sql
@@ -0,0 +1,3 @@
+insert into function file(02453_data.jsonl, TSV) select 1 settings engine_file_truncate_on_insert=1;
+select * from file(02453_data.jsonl, auto, 'x UInt32') settings input_format_allow_errors_num=1, input_format_record_errors_file_path='../error_file'; -- {serverError DATABASE_ACCESS_DENIED}
+