-
Notifications
You must be signed in to change notification settings - Fork 385
fix(parser): security vulnerabilities in DSN and SQL parsing #1553
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
base: dev
Are you sure you want to change the base?
Conversation
-Replaced `strncasecmp()` with manual comparison in `validate_key()` to prevent reading past buffer boundaries
> Added bounds checking for all string operations
- Maximum key length: 256 characters
- Maximum value length: 65536 characters
- Maximum DSN length: 4096 characters
Fixed uninitialized variable `start_pos` in `parse_conn_string()`
- Changed initial value from -1 to 0
- Added bounds checking before pointer arithmetic
Fixed handling of `{{` and `}}` in DSN values
- Previously treated as malformed input
- Now properly handled as escaped braces per ODBC specification
Added null checks for all input parameters
- Constructor validates `dsn`, `sql_str`, and hash table pointers
- `add_key_value_pair()` handles null value parameter
Added length validation before memory allocation
- Prevents excessive memory allocation from malformed input
|
@microsoft-github-policy-service agree |
|
@SajanGhimire1 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@SajanGhimire1 Thanks for raising the PR for this fix, Can you please check why is the pipeline failing for all OSs and fix it, also I would like you to add some tests to check if the code that you have added really works. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
For reference, here are the relevant build errors from these changes: macOSLinux (Ubuntu)Windows |
-Replaced
strncasecmp()with manual comparison invalidate_key()to prevent reading past buffer boundariesstart_posinparse_conn_string(){{and}}in DSN valuesdsn,sql_str, and hash table pointersadd_key_value_pair()handles null value parameter Added length validation before memory allocation