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

release-23.1: parser: validate privilege name when parsing #109706

Merged
merged 1 commit into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -6279,7 +6279,7 @@ privileges:
}
| privilege_list
{
privList, err := privilege.ListFromStrings($1.nameList().ToStrings())
privList, err := privilege.ListFromStrings($1.nameList().ToStrings(), privilege.OriginFromUserInput)
if err != nil {
return setErr(sqllex, err)
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/parser/testdata/alter_default_privileges
Original file line number Diff line number Diff line change
Expand Up @@ -942,3 +942,11 @@ ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE SELECT ON TABLES FROM foo, bar --
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE SELECT ON TABLES FROM foo, bar -- fully parenthesized
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE SELECT ON TABLES FROM foo, bar -- literals removed
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE SELECT ON TABLES FROM _, _ -- identifiers removed

error
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT CREATE, UNKNOWN_PRIV ON TYPES TO SESSION_USER WITH GRANT OPTION
----
at or near "on": syntax error: not a valid privilege: "unknown_priv"
DETAIL: source SQL:
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT CREATE, UNKNOWN_PRIV ON TYPES TO SESSION_USER WITH GRANT OPTION
^
8 changes: 8 additions & 0 deletions pkg/sql/parser/testdata/grant_revoke
Original file line number Diff line number Diff line change
Expand Up @@ -596,3 +596,11 @@ DETAIL: source SQL:
REVOKE SELECT ON ROLE foo, bar FROM blix
^
HINT: try \h REVOKE

error
GRANT CREATE, UNKNOWN_PRIV ON TABLE foo TO testuser
----
at or near "on": syntax error: not a valid privilege: "unknown_priv"
DETAIL: source SQL:
GRANT CREATE, UNKNOWN_PRIV ON TABLE foo TO testuser
^
32 changes: 26 additions & 6 deletions pkg/sql/privilege/privilege.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,18 +336,38 @@ func PrivilegesFromBitFields(
return ret, nil
}

// Origin indicates the origin of the privileges being parsed in
// ListFromStrings.
type Origin bool

const (
// OriginFromUserInput indicates that the privilege name came from user
// input and should be validated to make sure it refers to a real privilege.
OriginFromUserInput Origin = false

// OriginFromSystemTable indicates that the privilege name came from a
// system table and should be ignored if it does not refer to a real
// privilege.
OriginFromSystemTable Origin = true
)

// ListFromStrings takes a list of strings and attempts to build a list of Kind.
// We convert each string to uppercase and search for it in the ByName map.
// If an entry is not found in ByName, it is ignored.
func ListFromStrings(strs []string) (List, error) {
// If an entry is not found in ByName, it is either ignored or reports an error
// depending on the purpose.
func ListFromStrings(strs []string, origin Origin) (List, error) {
ret := make(List, len(strs))
for i, s := range strs {
k, ok := ByName[strings.ToUpper(s)]
if !ok {
// Ignore an unknown privilege name. This is so that it is possible to
// backport new privileges onto older release branches, without causing
// mixed-version compatibility issues.
continue
// Ignore an unknown privilege name if it came from a system table. This
// is so that it is possible to backport new privileges onto older release
// branches, without causing mixed-version compatibility issues.
if origin == OriginFromSystemTable {
continue
} else if origin == OriginFromUserInput {
return nil, errors.Errorf("not a valid privilege: %q", s)
}
}
ret[i] = k
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/syntheticprivilegecache/accumulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ func (s *accumulator) addRow(path, user tree.DString, privArr, grantOptionArr *t
for _, elem := range grantOptionArr.Array {
grantOptionStrings = append(grantOptionStrings, string(tree.MustBeDString(elem)))
}
privs, err := privilege.ListFromStrings(privilegeStrings)
privs, err := privilege.ListFromStrings(privilegeStrings, privilege.OriginFromSystemTable)
if err != nil {
return err
}
grantOptions, err := privilege.ListFromStrings(grantOptionStrings)
grantOptions, err := privilege.ListFromStrings(grantOptionStrings, privilege.OriginFromSystemTable)
if err != nil {
return err
}
Expand Down