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

INSERT ... RETURNING does not require a SELECT privilege [CORE6335] #6576

Closed
firebird-automations opened this issue Jun 17, 2020 · 5 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: @dyemanov

While UPDATE ... RETURNING and DELETE ... RETURNING require a SELECT privilege, INSERT ... RETURNING does not enforce that. It may look logical from the first glance, as there is usually no implicit cursor (that always exists for UPDATE/DELETE) and there's no OLD context for INSERT, so you can read only values from the row being inserted by yourself. However, some fields may be assigned implicitly (DEFAULT clause, GENERATED AS IDENTITY clause, BEFORE INSERT triggers) and the fact they can be read using the RETURNING clause looks like a minor security issue.

RETURNING is a non-standard extension, but the SQl specification includes <data change delta table> which is derived from rows changed by INSERT/UPDATE/DELETE statements, And it's explicitly specified that any column referenced in <data change delta table> require a SELECT permission on the target table for underlying INSERT/UPDATE/DELETE.

I suspect there may be a backward compatibility issue for those using INSERT ... RETURNING <generated PK> without a SELECT privilege granted. Thus backporting into v3 is questionable, I need other opinions in this regard.

Commits: 5e01527

====== Test Details ======

See functional/tabloid/dml-privileges-sufficiency.fbt

@firebird-automations
Copy link
Collaborator Author

Modified by: @dyemanov

assignee: Dmitry Yemanov [ dimitr ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @dyemanov

Fix Version: 4.0 RC 1 [ 10930 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

status: Open [ 1 ] => Open [ 1 ]

QA Status: No test => Covered by another test(s)

Test Details: See functional/tabloid/dml-privileges-sufficiency.fbt

@firebird-automations
Copy link
Collaborator Author

Commented by: @cincuranet

Probably good for 4.0 only. Given it has been like this for some versions, I don't think it's a good idea to backport to 3.0.

@firebird-automations
Copy link
Collaborator Author

Modified by: @dyemanov

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment