Bind values as query parameters; validate identifiers - #4
Merged
Merged
Conversation
Keys and values were formatted into SQL text (the delete did not even quote the key), and connection-string values were not brace-quoted. Values are now bound with ?, table/column names are validated and bracket-quoted, and connection-string values that could start a new attribute are brace-quoted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Every connection-string value is brace-quoted (a value already wrapped in braces could still inject attributes); DRIVER is passed bare. - Identifiers are delimited ([...] with ] doubled) instead of pattern- checked, so any name is one identifier; table names may be schema- qualified (dbo.person) and pre-bracketed names are accepted. - Parameters are passed as one tuple; container values/keys bind as their str() as before, never as pyodbc's parameter list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
1 participant
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.
Closes #3
?placeholders.[name]with]doubled, so any name is exactly one identifier; table names may be schema-qualified (dbo.person→[dbo].[person]); already-bracketed names are accepted. Empty/over-long/control-character names raiseValueError.}doubled), so no value (e.g. a password) can add connection attributes.Nonevalues now bind as NULL (previously the string'None'); list/dict/tuple values and keys still bind as theirstr().Behaviour for normal keys/values is unchanged (SQL Server converts bound parameter types as it did the literals). No fleet dependents.
Independent refute-review done; its findings (brace-wrapped values passed through unquoted, schema-qualified table names refused, pyodbc unpacking a lone tuple parameter) are fixed in the second commit.
Tests:
odbcdol/tests/test_injection.pyuses a stand-inpyodbcthat records what would be executed, so it runs without a database; 24 tests, all fail on master.🤖 Generated with Claude Code