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

sql: prevent arbitrary writes to system.comments #45712

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

knz
Copy link
Contributor

@knz knz commented Mar 4, 2020

Fixes #45707.

Previously, the GRANT, UPDATE, DELETE and INSERT privileges
were granted to public, i.e. everyone, on system.comments.

This was unintended - only users with permissions on an object
should be able to modify that object's comments.

This patch fixes it.

Release note (security update): Any user could previously modify any
database/table/view/index comment via direct SQL updates to
system.comments. This was unintended and a form of privilege
escalation, and is now prevented. The privileges required for the
COMMENT statement and pg_description, col_description(),
obj_description() and shobj_description() are operating as
in PostgreSQL and unaffected by this change: all users can view any
comments on any object (bypassing other privileges), but modifying
comments require write privilege on the target object.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz
Copy link
Contributor Author

knz commented Mar 4, 2020

Note that postgres allows users to bypass privileges to view comments from any object, this is also allowed by crdb and remains unchanged here.

@knz knz force-pushed the 20200304-comments branch 2 times, most recently from f686b07 to 09ffbe8 Compare March 4, 2020 16:42
Copy link
Contributor

@RichardJCai RichardJCai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @RichardJCai)

Copy link
Contributor

@RichardJCai RichardJCai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, forgot to add - current changes look good to me, should be okay once the failing logic tests are updated.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis)

Previously, the GRANT, UPDATE, DELETE and INSERT privileges
were granted to `public`, i.e. everyone, on `system.comments`.

This was unintended - only users with permissions on an object
should be able to modify that object's comments.

This patch fixes it.

Release note (security update): Any user could previously modify any
database/table/view/index comment via direct SQL updates to
`system.comments`. This was unintended and a form of privilege
escalation, and is now prevented. The privileges required for the
COMMENT statement and `pg_description`, `col_description()`,
`obj_description()` and `shobj_description()` are operating as in
PostgreSQL and unaffected by this change: all users can *view* any
comments on any object (bypassing other privileges), but modifying
comments require write privilege on the target object.
@knz knz force-pushed the 20200304-comments branch from 09ffbe8 to b7f8a96 Compare March 5, 2020 13:03
@knz
Copy link
Contributor Author

knz commented Mar 5, 2020

Thanks!

bors r=RichardJCai

@craig
Copy link
Contributor

craig bot commented Mar 5, 2020

Build failed (retrying...)

@craig
Copy link
Contributor

craig bot commented Mar 5, 2020

Build succeeded

@craig craig bot merged commit 4610ad2 into cockroachdb:master Mar 5, 2020
@knz knz deleted the 20200304-comments branch March 5, 2020 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sql: system.comments is mistakenly writable by all users
3 participants