Skip to content

Commit 4f8f2d6

Browse files
authored
Address CodeQL issues (#1615)
* Address CodeQL issues * Add lint fix
1 parent 15d7cfd commit 4f8f2d6

6 files changed

Lines changed: 26 additions & 13 deletions

File tree

CodeQL.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
queries:
2+
# Exclude all external PHP SDK source code
3+
- exclude:
4+
path:
5+
- "buildscripts/php-sdk"
6+
# Include only the SQLSRV and PDO_SQLSRV source files within the PHP SDK, where we drop them for compilation
7+
- include:
8+
path:
9+
- "buildscripts/php-sdk/**/sqlsrv"
10+
- "buildscripts/php-sdk/**/pdo_sqlsrv"

source/shared/core_sqlsrv.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1462,7 +1462,7 @@ struct sqlsrv_param
14621462

14631463
void copy_param_meta_ae(_Inout_ zval* param_z, _In_ param_meta_data& meta); // Only used when Always Encrypted is enabled
14641464

1465-
virtual ~sqlsrv_param(){ release_data(); }
1465+
virtual ~sqlsrv_param(){ sqlsrv_param::release_data(); }
14661466
virtual void release_data();
14671467

14681468
bool derive_string_types_sizes(_In_ zval* param_z);
@@ -1539,7 +1539,7 @@ struct sqlsrv_param_tvp : public sqlsrv_param
15391539
{
15401540
ZVAL_UNDEF(&placeholder_z);
15411541
}
1542-
virtual ~sqlsrv_param_tvp() { release_data(); }
1542+
virtual ~sqlsrv_param_tvp() { sqlsrv_param_tvp::release_data(); }
15431543
virtual void release_data();
15441544
virtual void bind_param(_Inout_ sqlsrv_stmt* stmt);
15451545
virtual void process_param(_Inout_ sqlsrv_stmt* stmt, _Inout_ zval* param_z);

source/shared/core_stmt.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2911,9 +2911,10 @@ void sqlsrv_param_inout::finalize_output_string()
29112911
char* outString = NULL;
29122912
SQLLEN outLen = 0;
29132913

2914-
// When encoding is UTF-8 or SYSTEM, ODBC returns data as UTF-16 wide characters
2915-
// in the output parameter buffer. ODBC guarantees proper alignment and valid UTF-16 data.
2916-
const SQLWCHAR* wide_str = reinterpret_cast<const SQLWCHAR*>(str);
2914+
// For UTF-8/SYSTEM encoding, the output parameter buffer was bound with SQL_C_WCHAR via
2915+
// SQLBindParameter, so ODBC wrote UTF-16 data into it. The reinterpret_cast is necessary
2916+
// because the ODBC API binds output params to a generic char* buffer.
2917+
const SQLWCHAR* wide_str = reinterpret_cast<const SQLWCHAR*>(str); // CodeQL [SM02986] buffer contains UTF-16 data from ODBC output parameter bound with SQL_C_WCHAR
29172918
bool result = convert_string_from_utf16(encoding, wide_str, int(str_len / sizeof(SQLWCHAR)), &outString, outLen);
29182919
CHECK_CUSTOM_ERROR(!result, stmt, SQLSRV_ERROR_OUTPUT_PARAM_ENCODING_TRANSLATE, get_last_error_message(), NULL) {
29192920
throw core::CoreException();

source/shared/core_stream.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,10 @@ size_t sqlsrv_stream_read(_Inout_ php_stream* stream, _Out_writes_bytes_(count)
200200
throw core::CoreException();
201201
}
202202

203-
// When encoding is UTF-8, SQLGetData fills temp_buf with UTF-16 wide character data (SQL_C_WCHAR).
204-
// The buffer is allocated as char* but contains properly aligned UTF-16 data from ODBC.
205-
// Reinterpret as LPCWSTR for conversion functions. This is safe because ODBC guarantees proper alignment.
206-
const LPCWSTR wide_buffer = reinterpret_cast<LPCWSTR>( temp_buf.get() );
203+
// temp_buf contains UTF-16 data written by the ODBC driver via SQLGetData with SQL_C_WCHAR
204+
// (set at line 88 when ss->encoding == CP_UTF8). The char* buffer is a generic allocation;
205+
// the reinterpret_cast is necessary because the ODBC API uses SQLPOINTER (void*) buffers.
206+
const LPCWSTR wide_buffer = reinterpret_cast<LPCWSTR>( temp_buf.get() ); // CodeQL [SM02986] buffer contains UTF-16 data from ODBC SQLGetData with SQL_C_WCHAR
207207
#ifndef _WIN32
208208
int enc_len = SystemLocale::FromUtf16( ss->encoding, wide_buffer,
209209
static_cast<int>(read >> 1), buf, static_cast<int>(count), NULL, NULL );

source/shared/core_util.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,11 @@ bool convert_string_from_utf16_inplace( _In_ SQLSRV_ENCODING encoding, _Inout_up
106106
char* outString = NULL;
107107
SQLLEN outLen = 0;
108108

109-
// The string buffer contains UTF-16 encoded data. Reinterpret as SQLWCHAR* for conversion.
110-
// This is safe because the caller ensures the buffer contains valid UTF-16 data.
111-
const SQLWCHAR* wide_str = reinterpret_cast<const SQLWCHAR*>(*string);
109+
// The buffer contains UTF-16 data written by the ODBC driver via SQLGetData with SQL_C_WCHAR.
110+
// The only caller (get_field_as_string in core_stmt.cpp) guards this call with
111+
// "if (c_type == SQL_C_WCHAR)", ensuring the buffer was populated as UTF-16 by the driver.
112+
// The reinterpret_cast is necessary because the ODBC API uses a generic char* buffer.
113+
const SQLWCHAR* wide_str = reinterpret_cast<const SQLWCHAR*>(*string); // CodeQL [SM02986] buffer is ensured to contain UTF-16 data
112114
bool result = convert_string_from_utf16( encoding, wide_str, int(len / sizeof(SQLWCHAR)), &outString, outLen );
113115

114116
if (result)

test/tools/mock_tds_server.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,7 @@ def generate_self_signed_cert(cert_path, key_path):
13411341
.public_key(key.public_key())
13421342
.serial_number(x509.random_serial_number())
13431343
.not_valid_before(datetime.datetime.now(datetime.timezone.utc))
1344-
.not_valid_after(datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(days=365))
1344+
.not_valid_after(datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(days=30))
13451345
.sign(key, hashes.SHA256())
13461346
)
13471347
with open(key_path, "wb") as f:

0 commit comments

Comments
 (0)