fix(parser): security vulnerabilities in DSN and SQL parsing #1553

SajanGhimire1 · 2026-01-08T13:47:12Z

Fix PDO DSN parser: remove invalid code and improve key validation

Removed invalid 'return true;' outside any function in anonymous namespace.
Replaced unsafe strncasecmp comparison with safe manual lowercase comparison in validate_key().
Initialized start_pos to 0 in parse_conn_string() to avoid negative indexing issues.
Minor code cleanup for safety and correctness in connection string and SQL string parsers.

-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

SajanGhimire1 · 2026-01-09T01:22:24Z

@microsoft-github-policy-service agree

jahnvi480 · 2026-01-14T09:44:21Z

/azp run

azure-pipelines · 2026-01-14T09:44:31Z

Azure Pipelines successfully started running 1 pipeline(s).

jahnvi480 · 2026-01-14T10:00:19Z

@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.

jahnvi480 · 2026-01-16T05:51:50Z

/azp run

azure-pipelines · 2026-01-16T05:52:01Z

Azure Pipelines successfully started running 1 pipeline(s).

David-Engel · 2026-01-16T18:23:32Z

For reference, here are the relevant build errors from these changes:

macOS

/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:158:78: error: no member named 'current_key_name' in 'string_parser'; did you mean 'current_key'?
  158 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_DSN_VALUE, this->current_key_name, nullptr);
      |                                                                              ^~~~~~~~~~~~~~~~
      |                                                                              current_key
/Users/runner/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:48: note: expanded from macro 'THROW_PDO_ERROR'
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                                                ^
/Users/runner/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:119:22: note: 'current_key' declared here
  119 |         unsigned int current_key;
      |                      ^
/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:192:36: error: use of undeclared identifier 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH'
  192 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^
/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:199:36: error: use of undeclared identifier 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH'
  199 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^
/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:292:52: error: use of undeclared identifier 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH'
  292 |                         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                                    ^

Linux (Ubuntu)

In file included from /home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:26:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp: In member function ‘void string_parser::add_key_value_pair(const char*, int)’:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:158:78: error: ‘class string_parser’ has no member named ‘current_key_name’; did you mean ‘current_key’?
  158 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_DSN_VALUE, this->current_key_name, nullptr);
      |                                                                              ^~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:48: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                                                ^~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp: In member function ‘void conn_string_parser::validate_key(const char*, int)’:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:192:36: error: ‘PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH’ was not declared in this scope; did you mean ‘PDO_SQLSRV_ERROR_INVALID_ENCODING’?
  192 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:30: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                              ^~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:199:36: error: ‘PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH’ was not declared in this scope; did you mean ‘PDO_SQLSRV_ERROR_INVALID_ENCODING’?
  199 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:30: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                              ^~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp: In member function ‘void conn_string_parser::parse_conn_string()’:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:292:52: error: ‘PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH’ was not declared in this scope; did you mean ‘PDO_SQLSRV_ERROR_INVALID_ENCODING’?
  292 |                         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:30: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                              ^~~~~~

Windows

ext\pdo_sqlsrv\pdo_parser.cpp(158): error C2039: 'current_key_name': is not a member of 'string_parser'
D:\a\1\s\buildscripts\php-sdk\phpdev\vs17\x64\php-8.4.16-src\ext\pdo_sqlsrv\php_pdo_sqlsrv_int.h(112): note: see declaration of 'string_parser'
ext\pdo_sqlsrv\pdo_parser.cpp(192): error C2065: 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH': undeclared identifier
ext\pdo_sqlsrv\pdo_parser.cpp(199): error C2065: 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH': undeclared identifier
ext\pdo_sqlsrv\pdo_parser.cpp(292): error C2065: 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH': undeclared identifier
pdo_util.cpp

- Removed invalid 'return true;' outside any function in anonymous namespace. - Replaced unsafe strncasecmp comparison with safe manual lowercase comparison in validate_key(). - Initialized start_pos to 0 in parse_conn_string() to avoid negative indexing issues. - Minor code cleanup for safety and correctness in connection string and SQL string parsers. These changes fix critical parsing bugs in PDO DSN handling and ensure robust validation of keys, values, and placeholders.

SajanGhimire1 · 2026-01-17T11:52:53Z

@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.

Thanks for the review, @jahnvi480! I’ll investigate the pipeline failures across all OSs and add tests to verify the changes.

SajanGhimire1 · 2026-01-17T11:53:58Z

For reference, here are the relevant build errors from these changes:

macOS

/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:158:78: error: no member named 'current_key_name' in 'string_parser'; did you mean 'current_key'?
  158 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_DSN_VALUE, this->current_key_name, nullptr);
      |                                                                              ^~~~~~~~~~~~~~~~
      |                                                                              current_key
/Users/runner/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:48: note: expanded from macro 'THROW_PDO_ERROR'
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                                                ^
/Users/runner/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:119:22: note: 'current_key' declared here
  119 |         unsigned int current_key;
      |                      ^
/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:192:36: error: use of undeclared identifier 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH'
  192 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^
/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:199:36: error: use of undeclared identifier 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH'
  199 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^
/Users/runner/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:292:52: error: use of undeclared identifier 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH'
  292 |                         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                                    ^

Linux (Ubuntu)

In file included from /home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:26:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp: In member function ‘void string_parser::add_key_value_pair(const char*, int)’:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:158:78: error: ‘class string_parser’ has no member named ‘current_key_name’; did you mean ‘current_key’?
  158 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_DSN_VALUE, this->current_key_name, nullptr);
      |                                                                              ^~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:48: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                                                ^~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp: In member function ‘void conn_string_parser::validate_key(const char*, int)’:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:192:36: error: ‘PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH’ was not declared in this scope; did you mean ‘PDO_SQLSRV_ERROR_INVALID_ENCODING’?
  192 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:30: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                              ^~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:199:36: error: ‘PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH’ was not declared in this scope; did you mean ‘PDO_SQLSRV_ERROR_INVALID_ENCODING’?
  199 |         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:30: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                              ^~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp: In member function ‘void conn_string_parser::parse_conn_string()’:
/home/vsts/work/1/s/source/pdo_sqlsrv/pdo_parser.cpp:292:52: error: ‘PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH’ was not declared in this scope; did you mean ‘PDO_SQLSRV_ERROR_INVALID_ENCODING’?
  292 |                         THROW_PDO_ERROR(this->ctx, PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH);
      |                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/vsts/work/1/s/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h:399:30: note: in definition of macro ‘THROW_PDO_ERROR’
  399 |     call_error_handler( ctx, custom, false, ## __VA_ARGS__ ); \
      |                              ^~~~~~

Windows

ext\pdo_sqlsrv\pdo_parser.cpp(158): error C2039: 'current_key_name': is not a member of 'string_parser'
D:\a\1\s\buildscripts\php-sdk\phpdev\vs17\x64\php-8.4.16-src\ext\pdo_sqlsrv\php_pdo_sqlsrv_int.h(112): note: see declaration of 'string_parser'
ext\pdo_sqlsrv\pdo_parser.cpp(192): error C2065: 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH': undeclared identifier
ext\pdo_sqlsrv\pdo_parser.cpp(199): error C2065: 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH': undeclared identifier
ext\pdo_sqlsrv\pdo_parser.cpp(292): error C2065: 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH': undeclared identifier
pdo_util.cpp

Thanks for sharing the build errors, @David-Engel! The issues with 'current_key_name' and 'PDO_SQLSRV_ERROR_INVALID_KEY_LENGTH' have been addressed in the latest fix — the parser now uses 'current_key' and proper key validation. I’ll make sure the pipeline passes on all OSs with added tests.

SajanGhimire1 · 2026-01-17T12:03:46Z

@SajanGhimire1 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree

jahnvi480 · 2026-01-19T08:17:17Z

/azp run

azure-pipelines · 2026-01-19T08:17:27Z

Azure Pipelines successfully started running 1 pipeline(s).

…laceholders

Merge branch 'dev' into patch-3

1e88107

jahnvi480 added 2 commits January 15, 2026 22:06

Merge branch 'dev' into patch-3

c4afdbf

Merge branch 'dev' into patch-3

4c99ce8

Merge branch 'dev' into patch-3

a085cc1

fix(pdo_parser): use safe DSN parsing with bounds checks and proper p…

49a0ed5

…laceholders

fix(parser): security vulnerabilities in DSN and SQL parsing #1553

Are you sure you want to change the base?

fix(parser): security vulnerabilities in DSN and SQL parsing #1553

Uh oh!

Conversation

SajanGhimire1 commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SajanGhimire1 commented Jan 9, 2026

Uh oh!

jahnvi480 commented Jan 14, 2026

Uh oh!

azure-pipelines bot commented Jan 14, 2026

Uh oh!

jahnvi480 commented Jan 14, 2026

Uh oh!

jahnvi480 commented Jan 16, 2026

Uh oh!

azure-pipelines bot commented Jan 16, 2026

Uh oh!

David-Engel commented Jan 16, 2026

macOS

Linux (Ubuntu)

Windows

Uh oh!

SajanGhimire1 commented Jan 17, 2026

Uh oh!

SajanGhimire1 commented Jan 17, 2026

macOS

Linux (Ubuntu)

Windows

Uh oh!

SajanGhimire1 commented Jan 17, 2026

Uh oh!

jahnvi480 commented Jan 19, 2026

Uh oh!

azure-pipelines bot commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SajanGhimire1 commented Jan 8, 2026 •

edited

Loading