MSSQL QuoteIdentifier does not escape ] characters, enabling SQL injection in all MSSQL operations#324
Conversation
…e `]` characters, enabling sql injection in all mssql operations. Escape closing brackets by replacing `]` with `]]` inside QuoteIdentifier, matching the MySQL/PostgreSQL escaping pattern.
paragon-review
left a comment
paragon-review
left a comment
There was a problem hiding this comment.
Review Summary
This review identified 1 issue in 2 files.
Confidence score: 5/5
- Low risk - No critical or high-priority issues found
Severity breakdown: Low: 1
2 files reviewed, 1 comment
Tip: @paragon-run <instructions> to chat with our agent or push fixes!
| // QuoteIdentifier for mssql queries | ||
| func QuoteIdentifier(id string) string { | ||
| return "[" + id + "]" | ||
| return "[" + strings.ReplaceAll(id, "]", "]]") + "]" |
There was a problem hiding this comment.
Security: No unit tests exist for QuoteIdentifier or QuoteValue
No unit tests exist for QuoteIdentifier or QuoteValue. Regressions to this SQL injection fix would go undetected. Add table-driven tests covering ], ]], empty string, and embedded [ inputs.
View Details
Location: pkg/clients/mssql/mssql.go (lines 124)
Analysis
No unit tests exist for QuoteIdentifier or QuoteValue
| What fails | There are no tests for QuoteIdentifier or QuoteValue in the mssql package, so regressions to this critical SQL injection fix would not be caught. |
| Result | No test files found — zero coverage for quoting functions. |
| Expected | Table-driven unit tests should cover edge cases: input containing ], input containing ]], empty string, embedded [ characters, and normal identifiers. |
| Impact | Without tests, future changes could silently reintroduce the SQL injection vulnerability this PR fixes. |
How to reproduce
ls pkg/clients/mssql/*_test.go # No test files existAI Fix Prompt
Fix this issue: No unit tests exist for QuoteIdentifier or QuoteValue. Regressions to this SQL injection fix would go undetected. Add table-driven tests covering ], ]], empty string, and embedded [ inputs.
Location: pkg/clients/mssql/mssql.go (lines 124)
Problem: There are no tests for QuoteIdentifier or QuoteValue in the mssql package, so regressions to this critical SQL injection fix would not be caught.
Current behavior: No test files found — zero coverage for quoting functions.
Expected: Table-driven unit tests should cover edge cases: input containing ], input containing ]], empty string, embedded [ characters, and normal identifiers.
Steps to reproduce: ls pkg/clients/mssql/*_test.go # No test files exist
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
|
Hey @crossplane-contrib! We ran Paragon on this PR and it caught some real issues — check the inline comments above. Found 1 issues including bugs and security fixes. We know maintaining OSS is tough, especially without automated code review catching things before they hit prod. We set up open source projects with Paragon so you don't have to worry about that stuff. Shoot me an email at shane@polarity.so if you're interested, happy to get you set up! |
2 participants
Description of your changes
Fixes #
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested