Skip to content

Commit 941e5b7

Browse files
gargsaumyaCopilot
gargsaumya
and
Copilot
committed
FIX: validate direct fetch indicators
Apply fixed-width and decimal indicator bounds before SQLGetData conversions used by fetchone and LOB-containing batches, with injected malformed-driver regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9718457 commit 941e5b7

2 files changed

Lines changed: 37 additions & 0 deletions

File tree

‎mssql_python/pybind/ddbc_bindings.cpp‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ size_t FixedFetchValueSize(SQLSMALLINT dataType) {
422422
return sizeof(SQL_TIMESTAMP_STRUCT);
423423
case SQL_TYPE_DATE:
424424
return sizeof(SQL_DATE_STRUCT);
425+
case SQL_TYPE_TIME:
425426
case SQL_SS_TIME2:
426427
return sizeof(SQL_SS_TIME2_STRUCT);
427428
case SQL_GUID:
@@ -440,6 +441,15 @@ void ValidateFixedFetchDataLength(SQLSMALLINT dataType, uint64_t dataLength) {
440441
}
441442
}
442443

444+
void ValidateDirectFetchDataLength(SQLRETURN ret, SQLSMALLINT dataType, SQLLEN indicator) {
445+
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
446+
if (indicator < 0) {
447+
ThrowStdException("Unexpected negative data length");
448+
}
449+
ValidateFixedFetchDataLength(dataType, static_cast<uint64_t>(indicator));
450+
}
451+
}
452+
443453
constexpr int MAX_NATIVE_ROW_COUNT = 1000000;
444454
constexpr size_t MAX_NATIVE_FETCH_BYTES = 256ULL * 1024 * 1024;
445455
constexpr size_t MAX_NATIVE_PARAMETER_BYTES = 256ULL * 1024 * 1024;
@@ -4275,6 +4285,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
42754285
SQLLEN indicator = 0;
42764286
ret = SQLGetData_ptr(hStmt, i, SQL_C_LONG, &intValue, 0, &indicator);
42774287
CaptureFetchDiagnostics(hStmt, ret, messages);
4288+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
42784289
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
42794290
row.append(static_cast<int>(intValue));
42804291
} else {
@@ -4287,6 +4298,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
42874298
SQLLEN indicator = 0;
42884299
ret = SQLGetData_ptr(hStmt, i, SQL_C_SHORT, &smallIntValue, 0, &indicator);
42894300
CaptureFetchDiagnostics(hStmt, ret, messages);
4301+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
42904302
if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
42914303
row.append(py::none());
42924304
break;
@@ -4306,6 +4318,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
43064318
SQLLEN indicator = 0;
43074319
ret = SQLGetData_ptr(hStmt, i, SQL_C_FLOAT, &realValue, 0, &indicator);
43084320
CaptureFetchDiagnostics(hStmt, ret, messages);
4321+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
43094322
if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
43104323
row.append(py::none());
43114324
break;
@@ -4330,6 +4343,14 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
43304343
CaptureFetchDiagnostics(hStmt, ret, messages);
43314344

43324345
if (SQL_SUCCEEDED(ret)) {
4346+
if (indicator == SQL_NULL_DATA) {
4347+
row.append(py::none());
4348+
break;
4349+
}
4350+
if (indicator < 0) {
4351+
ThrowStdException("Unexpected negative data length");
4352+
}
4353+
ValidateDecimalDataLength(static_cast<uint64_t>(indicator));
43334354
try {
43344355
// Validate 'indicator' to avoid buffer overflow and
43354356
// fallback to a safe null-terminated read when length
@@ -4386,6 +4407,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
43864407
SQLLEN indicator = 0;
43874408
ret = SQLGetData_ptr(hStmt, i, SQL_C_DOUBLE, &doubleValue, 0, &indicator);
43884409
CaptureFetchDiagnostics(hStmt, ret, messages);
4410+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
43894411
if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
43904412
row.append(py::none());
43914413
break;
@@ -4405,6 +4427,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
44054427
SQLLEN indicator = 0;
44064428
ret = SQLGetData_ptr(hStmt, i, SQL_C_SBIGINT, &bigintValue, 0, &indicator);
44074429
CaptureFetchDiagnostics(hStmt, ret, messages);
4430+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
44084431
if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
44094432
row.append(py::none());
44104433
break;
@@ -4425,6 +4448,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
44254448
ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_DATE, &dateValue, sizeof(dateValue),
44264449
&indicator);
44274450
CaptureFetchDiagnostics(hStmt, ret, messages);
4451+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
44284452
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
44294453
row.append(
44304454
FetchTemporal::date(dateValue.year, dateValue.month, dateValue.day));
@@ -4439,6 +4463,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
44394463
SQLLEN indicator = 0;
44404464
ret = SQLGetData_ptr(hStmt, i, SQL_C_SS_TIME2, &t2, sizeof(t2), &indicator);
44414465
CaptureFetchDiagnostics(hStmt, ret, messages);
4466+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
44424467
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
44434468
row.append(FetchTemporal::time(
44444469
t2.hour, t2.minute, t2.second, t2.fraction / 1000)); // ns to µs
@@ -4460,6 +4485,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
44604485
ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_TIMESTAMP, &timestampValue,
44614486
sizeof(timestampValue), &indicator);
44624487
CaptureFetchDiagnostics(hStmt, ret, messages);
4488+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
44634489
if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
44644490
row.append(py::none());
44654491
break;
@@ -4484,6 +4510,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
44844510
ret = SQLGetData_ptr(hStmt, i, SQL_C_SS_TIMESTAMPOFFSET, &dtoValue,
44854511
sizeof(dtoValue), &indicator);
44864512
CaptureFetchDiagnostics(hStmt, ret, messages);
4513+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
44874514
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
44884515
LOG("SQLGetData: Retrieved DATETIMEOFFSET for column %d - "
44894516
"%d-%d-%d %d:%d:%d, fraction_ns=%u, tz_hour=%d, "
@@ -4576,6 +4603,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
45764603
SQLLEN indicator = 0;
45774604
ret = SQLGetData_ptr(hStmt, i, SQL_C_TINYINT, &tinyIntValue, 0, &indicator);
45784605
CaptureFetchDiagnostics(hStmt, ret, messages);
4606+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
45794607
if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
45804608
row.append(py::none());
45814609
break;
@@ -4595,6 +4623,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
45954623
SQLLEN indicator = 0;
45964624
ret = SQLGetData_ptr(hStmt, i, SQL_C_BIT, &bitValue, 0, &indicator);
45974625
CaptureFetchDiagnostics(hStmt, ret, messages);
4626+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
45984627
if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
45994628
row.append(py::none());
46004629
break;
@@ -4616,6 +4645,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
46164645
ret =
46174646
SQLGetData_ptr(hStmt, i, SQL_C_GUID, &guidValue, sizeof(guidValue), &indicator);
46184647
CaptureFetchDiagnostics(hStmt, ret, messages);
4648+
ValidateDirectFetchDataLength(ret, effectiveDataType, indicator);
46194649

46204650
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
46214651
std::vector<char> guid_bytes(16);
@@ -5850,6 +5880,11 @@ py::object RunFetchValidationTest(const std::string& scenario) {
58505880
ValidateFixedFetchDataLength(SQL_INTEGER, sizeof(SQLINTEGER) - 1);
58515881
} else if (scenario == "oversized_fixed_indicator") {
58525882
ValidateFixedFetchDataLength(SQL_GUID, sizeof(SQLGUID) + 1);
5883+
} else if (scenario == "short_direct_fixed_indicator") {
5884+
ValidateDirectFetchDataLength(SQL_SUCCESS, SQL_TYPE_TIMESTAMP,
5885+
sizeof(SQL_TIMESTAMP_STRUCT) - 1);
5886+
} else if (scenario == "oversized_direct_decimal_indicator") {
5887+
ValidateDecimalDataLength(MAX_DIGITS_IN_NUMERIC);
58535888
} else if (scenario == "zero_arrow_rows") {
58545889
ValidateArrowFetchedRowCount(0, 1, 1);
58555890
} else if (scenario == "oversized_arrow_fetch") {

‎tests/test_010_pybind_functions.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ def test_arrow_batch_rejects_unsafe_size_before_handle_access(batch_size):
6565
("oversized_indicator", "exceeds the allocated fetch buffer"),
6666
("short_fixed_indicator", "does not match the bound buffer size"),
6767
("oversized_fixed_indicator", "does not match the bound buffer size"),
68+
("short_direct_fixed_indicator", "does not match the bound buffer size"),
69+
("oversized_direct_decimal_indicator", "Decimal data exceeds"),
6870
("zero_arrow_rows", "successful Arrow fetch with zero rows"),
6971
("oversized_arrow_fetch", "more rows than the allocated Arrow buffers"),
7072
("oversized_arrow_batch", "more rows than the allocated Arrow buffers"),

0 commit comments

Comments
 (0)