Skip to content

Commit 3f5e3cb

Browse files
Backport ClickHouse#88089 to 25.8: Add a URI normalization for the SOURCE grants filter.
1 parent 5d92144 commit 3f5e3cb

File tree

7 files changed

+61
-4
lines changed

7 files changed

+61
-4
lines changed

docs/en/sql-reference/statements/grant.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,23 @@ GRANT READ ON S3('s3://foo/.*') TO john
680680
GRANT READ ON S3('s3://bar/.*') TO john
681681
```
682682

683+
:::warning
684+
Source filter takes **regexp** as a parameter, so a grant
685+
`GRANT READ ON URL('http://www.google.com') TO john;`
686+
687+
will allow queries
688+
```sql
689+
SELECT * FROM url('https://www.google.com');
690+
SELECT * FROM url('https://www-google.com');
691+
```
692+
693+
because `.` is treated as an `Any Single Character` in the regexps.
694+
This may lead to potential vulnerability. The correct grant should be
695+
```sql
696+
GRANT READ ON URL('https://www\.google\.com') TO john;
697+
```
698+
:::
699+
683700
**Re-granting with GRANT OPTION:**
684701

685702
If the original grant has `WITH GRANT OPTION`, it can be re-granted using `GRANT CURRENT GRANTS`:

src/TableFunctions/ITableFunction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ StoragePtr ITableFunction::execute(const ASTPtr & ast_function, ContextPtr conte
4444
if (is_insert_query)
4545
type_to_check = AccessType::WRITE;
4646

47-
context->getAccess()->checkAccessWithFilter(type_to_check, toStringSource(*access_object), getFunctionURI());
47+
context->getAccess()->checkAccessWithFilter(type_to_check, toStringSource(*access_object), getFunctionURINormalized());
4848
}
4949

5050
auto table_function_properties = TableFunctionFactory::instance().tryGetProperties(getName());

src/TableFunctions/ITableFunction.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,29 @@ class ITableFunction : public std::enable_shared_from_this<ITableFunction>
106106
/// For example for s3Cluster the database storage name is S3Cluster, and we need to check
107107
/// privileges as if it was S3.
108108
virtual const char * getNonClusteredStorageEngineName() const;
109+
110+
protected:
109111
/// The URI of function for permission checking. Can be empty string if not applicable.
110112
/// For example for url('https://foo.bar') URI would be 'https://foo.bar'.
111113
virtual const String & getFunctionURI() const
112114
{
113115
static const String empty;
114116
return empty;
115117
}
118+
119+
String getFunctionURINormalized() const
120+
{
121+
try
122+
{
123+
Poco::URI uri(getFunctionURI());
124+
uri.normalize();
125+
return uri.toString();
126+
}
127+
catch (const Poco::Exception &)
128+
{
129+
return "";
130+
}
131+
}
116132
};
117133

118134
/// Properties of table function that are independent of argument types and parameters.

src/TableFunctions/ITableFunctionXDBC.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ class ITableFunctionXDBC : public ITableFunction
6262

6363
void startBridgeIfNot(ContextPtr context) const;
6464

65-
const String & getFunctionURI() const override { return connection_string; }
66-
6765
String connection_string;
6866
String schema_name;
6967
String remote_table_name;

src/TableFunctions/TableFunctionURL.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ ColumnsDescription TableFunctionURL::getActualTableStructure(ContextPtr context,
143143
ColumnsDescription columns;
144144

145145
if (const auto access_object = getSourceAccessObject())
146-
context->getAccess()->checkAccessWithFilter(AccessType::READ, toStringSource(*access_object), getFunctionURI());
146+
context->getAccess()->checkAccessWithFilter(AccessType::READ, toStringSource(*access_object), getFunctionURINormalized());
147147
if (format == "auto")
148148
{
149149
columns = StorageURL::getTableStructureAndFormatFromData(

tests/queries/0_stateless/03636_normalize_url_in_source_grants.reference

Whitespace-only changes.
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#!/usr/bin/env bash
2+
# Tags: no-fasttest
3+
4+
CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
5+
# shellcheck source=../shell_config.sh
6+
. "$CUR_DIR"/../shell_config.sh
7+
8+
user="user03636_${CLICKHOUSE_DATABASE}_$RANDOM"
9+
10+
${CLICKHOUSE_CLIENT} <<EOF
11+
-- Cleanup
12+
DROP USER IF EXISTS $user;
13+
CREATE USER $user;
14+
GRANT CREATE TEMPORARY TABLE ON *.* TO $user;
15+
EOF
16+
17+
${CLICKHOUSE_CLIENT} --query "GRANT READ ON S3('http://localhost:11111/test/.*') TO $user WITH GRANT OPTION";
18+
19+
${CLICKHOUSE_CLIENT} --user $user --query "SELECT * FROM s3('http://localhost:11111/test/a.tsv', 'TSV') FORMAT Null;";
20+
${CLICKHOUSE_CLIENT} --user $user --query "SELECT * FROM s3('http://localhost:11111/a.tsv', 'TSV') FORMAT Null; -- { serverError ACCESS_DENIED }";
21+
${CLICKHOUSE_CLIENT} --user $user --query "SELECT * FROM s3('http://localhost:11111/test/../a.tsv', 'TSV') FORMAT Null; -- { serverError ACCESS_DENIED }";
22+
${CLICKHOUSE_CLIENT} --user $user --query "SELECT * FROM s3('http://localhost:11111/test/%2e%2e/a.tsv', 'TSV') FORMAT Null; -- { serverError ACCESS_DENIED }";
23+
24+
${CLICKHOUSE_CLIENT} <<EOF
25+
DROP USER IF EXISTS $user;
26+
EOF

0 commit comments

Comments
 (0)