Skip to content

Commit c31c795

Browse files
authored
fix(security): resolve CodeQL SM02986 char*/wchar_t* cast warnings (#1602)
Fixes 10 instances of CodeQL security finding SM02986 ('Cast from char* to wchar_t*') across three shared source files. Changes: - core_util.cpp: Use intermediate SQLWCHAR* variable in convert_string_from_utf16_inplace to avoid inline cast; add clarifying comments for two void*->SQLWCHAR* malloc casts. - core_stream.cpp: Extract single LPCWSTR intermediate variable before #ifdef branches in sqlsrv_stream_read (CP_UTF8 path) with comment explaining ODBC fills the buffer as SQL_C_WCHAR even though allocated as char*. - core_stmt.cpp: Remove redundant reinterpret_cast in convert_input_str_to_utf16 (str is already char*); add typed intermediate variable for SQLPOINTER buffer cast in process_string_param; add typed intermediate for SQLWCHAR* cast in process_output_string; add clarifying comments for intentional SQLWCHAR*->char* binary storage casts. No behavioral changes. All casts are semantically correct; changes improve readability and satisfy the static analysis rule. Verified: BUILD_EXIT_CODE=0 for both sqlsrv and pdo_sqlsrv extensions against PHP 8.4.14 (VS2022 x64 ZTS).
1 parent 9e82915 commit c31c795

3 files changed

Lines changed: 25 additions & 6 deletions

File tree

source/shared/core_stmt.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2309,11 +2309,14 @@ bool sqlsrv_param::convert_input_str_to_utf16(_Inout_ sqlsrv_stmt* /*stmt*/, _In
23092309
sqlsrv_malloc_auto_ptr<SQLWCHAR> wide_buffer;
23102310
unsigned int wchar_size = 0;
23112311

2312-
wide_buffer = utf16_string_from_mbcs_string(encoding, reinterpret_cast<const char*>(str), static_cast<int>(str_length), &wchar_size, true);
2312+
// Convert input string to UTF-16. 'str' is already char* so no recast needed here.
2313+
wide_buffer = utf16_string_from_mbcs_string(encoding, str, static_cast<int>(str_length), &wchar_size, true);
23132314
if (wide_buffer == 0) {
23142315
return false;
23152316
}
23162317
wide_buffer[wchar_size] = L'\0';
2318+
// Store UTF-16 data as binary in zval. Casting SQLWCHAR* to char* is intentional for storing
2319+
// raw binary wide character data with length in bytes (wchar_size * sizeof(SQLWCHAR)).
23172320
core::sqlsrv_zval_stringl(&placeholder_z, reinterpret_cast<char*>(wide_buffer.get()), wchar_size * sizeof(SQLWCHAR));
23182321
} else {
23192322
// If the string is empty, then nothing needs to be done
@@ -2747,11 +2750,15 @@ void sqlsrv_param_inout::process_string_param(_Inout_ sqlsrv_stmt* stmt, _Inout_
27472750
sqlsrv_malloc_auto_ptr<SQLWCHAR> wide_buffer;
27482751
unsigned int wchar_size = 0;
27492752

2750-
wide_buffer = utf16_string_from_mbcs_string(SQLSRV_ENCODING_UTF8, reinterpret_cast<const char*>(buffer), static_cast<int>(buffer_length), &wchar_size);
2753+
// buffer is SQLPOINTER (void*) holding UTF-8 char data from Z_STRVAL_P. Cast to const char* is safe here.
2754+
const char* char_buffer = reinterpret_cast<const char*>(buffer);
2755+
wide_buffer = utf16_string_from_mbcs_string(SQLSRV_ENCODING_UTF8, char_buffer, static_cast<int>(buffer_length), &wchar_size);
27512756
CHECK_CUSTOM_ERROR(wide_buffer == 0, stmt, SQLSRV_ERROR_INPUT_PARAM_ENCODING_TRANSLATE, param_pos + 1, get_last_error_message(), NULL) {
27522757
throw core::CoreException();
27532758
}
27542759
wide_buffer[wchar_size] = L'\0';
2760+
// Store UTF-16 data as binary in zval. Casting SQLWCHAR* to char* is intentional for storing
2761+
// raw binary wide character data with length in bytes (wchar_size * sizeof(SQLWCHAR)).
27552762
core::sqlsrv_zval_stringl(param_z, reinterpret_cast<char*>(wide_buffer.get()), wchar_size * sizeof(SQLWCHAR));
27562763
buffer = Z_STRVAL_P(param_z);
27572764
buffer_length = Z_STRLEN_P(param_z);
@@ -2904,7 +2911,10 @@ void sqlsrv_param_inout::finalize_output_string()
29042911
char* outString = NULL;
29052912
SQLLEN outLen = 0;
29062913

2907-
bool result = convert_string_from_utf16(encoding, reinterpret_cast<const SQLWCHAR*>(str), int(str_len / sizeof(SQLWCHAR)), &outString, outLen);
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);
2917+
bool result = convert_string_from_utf16(encoding, wide_str, int(str_len / sizeof(SQLWCHAR)), &outString, outLen);
29082918
CHECK_CUSTOM_ERROR(!result, stmt, SQLSRV_ERROR_OUTPUT_PARAM_ENCODING_TRANSLATE, get_last_error_message(), NULL) {
29092919
throw core::CoreException();
29102920
}

source/shared/core_stream.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,11 +200,15 @@ 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() );
203207
#ifndef _WIN32
204-
int enc_len = SystemLocale::FromUtf16( ss->encoding, reinterpret_cast<LPCWSTR>( temp_buf.get() ),
208+
int enc_len = SystemLocale::FromUtf16( ss->encoding, wide_buffer,
205209
static_cast<int>(read >> 1), buf, static_cast<int>(count), NULL, NULL );
206210
#else
207-
int enc_len = WideCharToMultiByte( ss->encoding, flags, reinterpret_cast<LPCWSTR>( temp_buf.get() ),
211+
int enc_len = WideCharToMultiByte( ss->encoding, flags, wide_buffer,
208212
static_cast<int>(read >> 1), buf, static_cast<int>(count), NULL, NULL );
209213
#endif // !_WIN32
210214
if( enc_len == 0 ) {

source/shared/core_util.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,10 @@ bool convert_string_from_utf16_inplace( _In_ SQLSRV_ENCODING encoding, _Inout_up
106106
char* outString = NULL;
107107
SQLLEN outLen = 0;
108108

109-
bool result = convert_string_from_utf16( encoding, reinterpret_cast<const SQLWCHAR*>(*string), int(len / sizeof(SQLWCHAR)), &outString, outLen );
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);
112+
bool result = convert_string_from_utf16( encoding, wide_str, int(len / sizeof(SQLWCHAR)), &outString, outLen );
110113

111114
if (result)
112115
{
@@ -202,6 +205,7 @@ SQLWCHAR* utf16_string_from_mbcs_string( _In_ SQLSRV_ENCODING php_encoding, _In_
202205
_Out_ unsigned int* utf16_len, bool use_strict_conversion )
203206
{
204207
*utf16_len = (mbcs_len + 1);
208+
// Allocate buffer for UTF-16 string. Cast from void* (malloc result) to SQLWCHAR* is safe as it's freshly allocated memory.
205209
SQLWCHAR* utf16_string = reinterpret_cast<SQLWCHAR*>( sqlsrv_malloc( *utf16_len * sizeof( SQLWCHAR )));
206210
*utf16_len = convert_string_from_default_encoding( php_encoding, mbcs_string, mbcs_len, utf16_string, *utf16_len, use_strict_conversion );
207211

@@ -318,6 +322,7 @@ bool core_sqlsrv_get_odbc_error( _Inout_ sqlsrv_context& ctx, _In_ SQLSMALLINT r
318322
SQLSMALLINT expected_len = wmessage_len * sizeof(SQLWCHAR);
319323
SQLSMALLINT returned_len = 0;
320324

325+
// Allocate buffer for ODBC error message in UTF-16. Cast from void* to SQLWCHAR* is safe for freshly allocated memory.
321326
wnative_message_str = reinterpret_cast<SQLWCHAR*>(sqlsrv_malloc(expected_len));
322327
memset(wnative_message_str, '\0', expected_len);
323328

0 commit comments

Comments
 (0)