Skip to content

Commit 0e0adf7

Browse files
committed
ODBC-479 Using SQLGetData with column > column count could crash
the driver. Actually in most cases. The check stood after the place where certain arrays(storting data on offets to start reading the data from) were read using that index - basically it could read past allocated area.
1 parent 74d6447 commit 0e0adf7

File tree

3 files changed

+18
-8
lines changed

3 files changed

+18
-8
lines changed

ma_statement.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2989,8 +2989,7 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT StatementHandle,
29892989
IrdRec= MADB_DescGetInternalRecord(Stmt->Ird, Offset, MADB_DESC_READ);
29902990
if (!IrdRec)
29912991
{
2992-
MADB_SetError(&Stmt->Error, MADB_ERR_07009, NULL, 0);
2993-
return Stmt->Error.ReturnValue;
2992+
return MADB_SetError(&Stmt->Error, MADB_ERR_07009, NULL, 0);
29942993
}
29952994

29962995
switch (TargetType) {

odbc_3_api.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1563,6 +1563,7 @@ SQLRETURN SQL_API SQLGetData(SQLHSTMT StatementHandle,
15631563
MADB_Stmt *Stmt= (MADB_Stmt*)StatementHandle;
15641564
unsigned int i;
15651565
MADB_DescRecord *IrdRec;
1566+
unsigned int columnCount= 0;
15661567

15671568
if (StatementHandle== SQL_NULL_HSTMT)
15681569
return SQL_INVALID_HANDLE;
@@ -1581,6 +1582,12 @@ SQLRETURN SQL_API SQLGetData(SQLHSTMT StatementHandle,
15811582
return MADB_GetBookmark(Stmt, TargetType, TargetValuePtr, BufferLength, StrLen_or_IndPtr);
15821583
}
15831584

1585+
/* This parameter validation has to be done before using it as array index */
1586+
if (Col_or_Param_Num > (columnCount= mysql_stmt_field_count(Stmt->stmt)))
1587+
{
1588+
return MADB_SetError(&Stmt->Error, MADB_ERR_07009, NULL, 0);
1589+
}
1590+
15841591
/* We don't need this to be checked in case of "internal" use of the GetData, i.e. for internal needs we should always get the data */
15851592
if ( Stmt->CharOffset[Col_or_Param_Num - 1] > 0
15861593
&& Stmt->CharOffset[Col_or_Param_Num - 1] >= Stmt->Lengths[Col_or_Param_Num - 1])
@@ -1594,7 +1601,7 @@ SQLRETURN SQL_API SQLGetData(SQLHSTMT StatementHandle,
15941601
}
15951602

15961603
/* reset offsets for other columns. Doing that here since "internal" calls should not do that */
1597-
for (i=0; i < mysql_stmt_field_count(Stmt->stmt); i++)
1604+
for (i=0; i < columnCount; i++)
15981605
{
15991606
if (i != Col_or_Param_Num - 1)
16001607
{

test/param.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
3-
2013, 2024 MariaDB Corporation AB
3+
2013, 2025 MariaDB Corporation AB
44
55
The MySQL Connector/ODBC is licensed under the terms of the GPLv2
66
<http://www.gnu.org/licenses/old-licenses/gpl-2.0.html>, like most
@@ -988,14 +988,16 @@ ODBC_TEST(t_bug59772)
988988
#undef ROWS_TO_INSERT
989989
}
990990

991-
991+
/* Enhanced it to also cover ODBC-479 SQLGetData with non-existent column index could crash
992+
the driver */
992993
ODBC_TEST(t_odbcoutparams)
993994
{
994995
SQLSMALLINT ncol, i;
995996
SQLINTEGER par[3]= {10, 20, 30}, val;
996997
SQLLEN len;
997998
SQLSMALLINT type[]= {SQL_PARAM_INPUT, SQL_PARAM_OUTPUT, SQL_PARAM_INPUT_OUTPUT};
998999
SQLCHAR str[20]= "initial value", buff[20];
1000+
double dummy= .0;
9991001

10001002
OK_SIMPLE_STMT(Stmt, "DROP PROCEDURE IF EXISTS t_odbcoutparams");
10011003
OK_SIMPLE_STMT(Stmt, "CREATE PROCEDURE t_odbcoutparams("
@@ -1006,8 +1008,6 @@ ODBC_TEST(t_odbcoutparams)
10061008
" SET p_in = p_in*10, p_out = (p_in+p_inout)*10, p_inout = p_inout*10; "
10071009
"END");
10081010

1009-
1010-
10111011
for (i=0; i < sizeof(par)/sizeof(SQLINTEGER); ++i)
10121012
{
10131013
CHECK_STMT_RC(Stmt, SQLBindParameter(Stmt, i+1, type[i], SQL_C_LONG, SQL_INTEGER, 0,
@@ -1024,8 +1024,12 @@ ODBC_TEST(t_odbcoutparams)
10241024

10251025
/* Only 1 row always - we still can get them as a result */
10261026
CHECK_STMT_RC(Stmt, SQLFetch(Stmt));
1027+
EXPECT_STMT(Stmt, SQLGetData(Stmt, 133, SQL_C_DOUBLE, &dummy, sizeof(dummy), NULL), SQL_ERROR);
1028+
CHECK_SQLSTATE(Stmt, "07009");
10271029
is_num(my_fetch_int(Stmt, 1), 1300);
10281030
is_num(my_fetch_int(Stmt, 2), 300);
1031+
EXPECT_STMT(Stmt, SQLGetData(Stmt, 3, SQL_C_DOUBLE, &dummy, sizeof(dummy), NULL), SQL_ERROR);
1032+
CHECK_SQLSTATE(Stmt, "07009");
10291033
FAIL_IF(SQLFetch(Stmt) != SQL_NO_DATA_FOUND, "eof expected");
10301034

10311035
CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
@@ -1727,7 +1731,7 @@ MA_ODBC_TESTS my_tests[]=
17271731
{t_bug49029, "t_bug49029"},
17281732
{t_bug56804, "t_bug56804"},
17291733
{t_bug59772, "t_bug59772"},
1730-
{t_odbcoutparams, "t_odbcoutparams"},
1734+
{t_odbcoutparams, "t_odbcoutparams-with_odbc-479"},
17311735
{t_bug14501952, "t_bug14501952"},
17321736
{t_bug14563386, "t_bug14563386"},
17331737
{t_bug14551229, "t_bug14551229"},

0 commit comments

Comments
 (0)