Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insufficient access control checks in system.metadata.table_comments when redirections enabled #18514

Closed
findepi opened this issue Aug 3, 2023 · 4 comments · Fixed by #18650
Labels
bug Something isn't working needs-docs This pull request requires changes to the documentation

Comments

@findepi
Copy link
Member

findepi commented Aug 3, 2023

the code

private Optional<String> getComment(
Session session,
QualifiedTablePrefix prefix,
SchemaTableName name,
Map<SchemaTableName, ViewInfo> views,
Map<SchemaTableName, ViewInfo> materializedViews)
{
ViewInfo materializedViewDefinition = materializedViews.get(name);
if (materializedViewDefinition != null) {
return materializedViewDefinition.getComment();
}
ViewInfo viewInfo = views.get(name);
if (viewInfo != null) {
return viewInfo.getComment();
}
QualifiedObjectName tableName = new QualifiedObjectName(prefix.getCatalogName(), name.getSchemaName(), name.getTableName());
return metadata.getRedirectionAwareTableHandle(session, tableName).tableHandle()
.map(handle -> metadata.getTableMetadata(session, handle))
.map(metadata -> metadata.getMetadata().getComment())
.orElseGet(() -> {
// A previously listed table might have been dropped concurrently
LOG.warn("Failed to get metadata for table: %s", name);
return Optional.empty();
});

doesn't consult AccessControl for redirected table.
non-redirected tables are passed thru io.trino.security.AccessControl#filterTables inside io.trino.metadata.MetadataListing#listTables.

@findepi findepi added the bug Something isn't working label Aug 3, 2023
@findepi
Copy link
Member Author

findepi commented Aug 3, 2023

@lukasz-walkiewicz can you consider picking this up?

@lukasz-walkiewicz
Copy link
Member

I can take a look.

@findepi
Copy link
Member Author

findepi commented Aug 4, 2023

@mosabua @colebow i can't find it. do the docs say whether access control is consulted before redirection, after, or both?

@mosabua
Copy link
Member

mosabua commented Aug 8, 2023

I dont think we have any security related info about redirection

@findepi findepi added the needs-docs This pull request requires changes to the documentation label Aug 9, 2023
@findepi findepi changed the title Insufficient access control checks in system.metadata.table_comments when redirections enabled Insufficient access control checks in metadata tables when redirections enabled Aug 18, 2023
@findepi findepi changed the title Insufficient access control checks in metadata tables when redirections enabled Insufficient access control checks in system.metadata.table_comments when redirections enabled Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-docs This pull request requires changes to the documentation
Development

Successfully merging a pull request may close this issue.

3 participants