Merge pull request ClickHouse#87730 from ClickHouse/backport/25.8/87545

Backport ClickHouse#87545 to 25.8: Better access validation for `Buffer` engine.

Author: clickhouse-gh[bot]

src/Storages/StorageBuffer.cpp

@@ -1,5 +1,6 @@
 #include <Storages/StorageBuffer.h>
 
+#include <Access/Common/AccessFlags.h>
 #include <Analyzer/TableNode.h>
 #include <Analyzer/Utils.h>
 #include <Interpreters/Context.h>
@@ -290,6 +291,7 @@ void StorageBuffer::read(
 
     if (auto destination = getDestinationTable())
     {
+        local_context->checkAccess(AccessType::SELECT, destination->getStorageID(), column_names);
         auto destination_lock
             = destination->lockForShare(local_context->getCurrentQueryId(), local_context->getSettingsRef()[Setting::lock_acquire_timeout]);
 
@@ -642,13 +644,14 @@ static void appendBlock(LoggerPtr log, const Block & from, Block & to)
 }
 
 
-class BufferSink : public SinkToStorage
+class BufferSink : public SinkToStorage, WithContext
 {
 public:
     explicit BufferSink(
         StorageBuffer & storage_,
-        const StorageMetadataPtr & metadata_snapshot_)
-        : SinkToStorage(std::make_shared<const Block>(metadata_snapshot_->getSampleBlock()))
+        const StorageMetadataPtr & metadata_snapshot_,
+        const ContextPtr & context_)
+        : SinkToStorage(std::make_shared<const Block>(metadata_snapshot_->getSampleBlock())), WithContext(context_)
         , storage(storage_)
         , metadata_snapshot(metadata_snapshot_)
     {
@@ -669,6 +672,7 @@ class BufferSink : public SinkToStorage
         StoragePtr destination = storage.getDestinationTable();
         if (destination)
         {
+            getContext()->checkAccess(AccessType::INSERT, destination->getStorageID());
             destination = DatabaseCatalog::instance().tryGetTable(storage.destination_id, storage.getContext());
             if (destination.get() == &storage)
                 throw Exception(ErrorCodes::INFINITE_LOOP, "Destination table is myself. Write will cause infinite loop.");
@@ -766,9 +770,9 @@ class BufferSink : public SinkToStorage
 };
 
 
-SinkToStoragePtr StorageBuffer::write(const ASTPtr & /*query*/, const StorageMetadataPtr & metadata_snapshot, ContextPtr /*context*/, bool /*async_insert*/)
+SinkToStoragePtr StorageBuffer::write(const ASTPtr & /*query*/, const StorageMetadataPtr & metadata_snapshot, ContextPtr local_context, bool /*async_insert*/)
 {
-    return std::make_shared<BufferSink>(*this, metadata_snapshot);
+    return std::make_shared<BufferSink>(*this, metadata_snapshot, local_context);
 }
 
 

tests/queries/0_stateless/03631_buffer_access_check.reference

@@ -0,0 +1,3 @@
+OK
+OK
+foo

tests/queries/0_stateless/03631_buffer_access_check.sh

@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+# Tags: long, no-replicated-database, no-async-insert
+
+CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+# shellcheck source=../shell_config.sh
+. "$CURDIR"/../shell_config.sh
+
+
+user="user03631_${CLICKHOUSE_DATABASE}_$RANDOM"
+db=${CLICKHOUSE_DATABASE}
+
+${CLICKHOUSE_CLIENT} <<EOF
+CREATE TABLE $db.test_table (s String) ENGINE = MergeTree ORDER BY s;
+INSERT INTO $db.test_table VALUES ('foo');
+
+DROP USER IF EXISTS $user;
+CREATE USER $user;
+GRANT SELECT, CREATE, INSERT ON $db.test_buffer TO $user;
+GRANT TABLE ENGINE ON Buffer TO $user;
+EOF
+
+${CLICKHOUSE_CLIENT} --user $user --query "CREATE TABLE $db.test_buffer ENGINE = Buffer($db, test_table, 1, 10, 100, 10000, 1000000, 10000000, 100000000)"
+(( $(${CLICKHOUSE_CLIENT} --user $user --query "SELECT * FROM $db.test_buffer" 2>&1 | grep -c "Not enough privileges") >= 1 )) && echo "OK" || echo "UNEXPECTED"
+(( $(${CLICKHOUSE_CLIENT} --user $user --query "INSERT INTO $db.test_buffer VALUES ('bar')" 2>&1 | grep -c "Not enough privileges") >= 1 )) && echo "OK" || echo "UNEXPECTED"
+
+${CLICKHOUSE_CLIENT} --query "GRANT SELECT ON $db.test_table TO $user"
+${CLICKHOUSE_CLIENT} --user $user --query "SELECT * FROM $db.test_buffer"
+
+${CLICKHOUSE_CLIENT} --query "DROP USER IF EXISTS $user"