d1 import: block DEFAULT/REFERENCES SQL injection into executed DDL#1304
Draft
mscoutermarsh wants to merge 3 commits into
Draft
d1 import: block DEFAULT/REFERENCES SQL injection into executed DDL#1304mscoutermarsh wants to merge 3 commits into
mscoutermarsh wants to merge 3 commits into
Conversation
Unrecognized DEFAULT expressions on typed columns were spliced into CREATE TABLE as-is, so a payload like DEFAULT (0)); DROP TABLE users; -- could escape the statement and run via psql. Restrict those defaults to validated numeric/boolean/quoted literals (or omit them), harden UUID and quoted-literal handling, and drop unsafe REFERENCES action tails the same way. Co-authored-by: Mike Coutermarsh <coutermarsh.mike@gmail.com>
parseTableBody used the last ')' in RawDDL, so a dump that smuggled "); DROP ...; CREATE TABLE ..." after a real REFERENCES close pulled attacker SQL into the column/FK fragment. Match the opening paren instead, truncate RawDDL at that close during ParseDump, and cover the reported REFERENCES-tail injection end-to-end. Co-authored-by: Mike Coutermarsh <coutermarsh.mike@gmail.com>
assertValidPostgresDDL calls t.Skip when no local Postgres is available, which skipped later unit assertions in the DEFAULT injection regression. Co-authored-by: Mike Coutermarsh <coutermarsh.mike@gmail.com>
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
Two related D1-import SQL injection paths let attacker-controlled dump fragments reach
psql -fwith the destinationpostgresrole:convertDefaultechoed unrecognized typed defaults verbatim, soDEFAULT (0)); DROP TABLE users; --could escapeCREATE TABLE.convertReferencesClauseechoed the action tail verbatim, andparseTableBodyused the last)in RawDDL, so a dump likeREFERENCES a(id) ON DELETE CASCADE); DROP TABLE users; CREATE TABLE dummy (z int );pulled injected statements into the FK fragment and into executed DDL.Fix
;/ extra parentheses.matchingParenEnd(notLastIndex), and truncate RawDDL at that close duringParseDump.Tests
parseTableBodyignores SQL after the balanced closego test ./internal/import/d1/passes.