[HIGH] Prevent SQL injection through embed_params type names - #744
Open
OskarEichler wants to merge 1 commit into
Open
[HIGH] Prevent SQL injection through embed_params type names#744OskarEichler wants to merge 1 commit into
OskarEichler wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Quote type-name components before
PG::Connection#embed_paramsinserts them into generated SQL. This prevents caller-supplied:typenamevalues (or mutable coder names) from terminating the cast and appending an additional SQL statement.Urgency: HIGH. A caller that lets untrusted metadata reach
:typenameand then executes the generated SQL can run arbitrary SQL with the connection's database privileges.embed_paramsis documented primarily for debugging, which limits expected exposure, but its result is executable SQL and the examples/tests execute it.Reproduction
On current
master, this input:produces:
A local PostgreSQL reproduction completed in 0.252 seconds, confirming that the appended statement ran.
After this change, unsafe identifier components are quoted and PostgreSQL rejects the invalid type immediately (0.000 seconds in the same focused reproduction). Ordinary names such as
int8,int[], and schema-qualified names retain their existing generated form.Verification
rbenv exec ruby -c lib/pg/connection.rbrbenv exec bundle exec rspec spec/pg/connection_spec.rb:3057— 20 examples, 0 failuresgit diff --checkScope and compatibility
master; it is not part of the installed 1.6.3 release reviewed downstream.:typenameas raw SQL syntax.