Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add dynamic buffers in GetDiagRecs and GetErrorFromHandle and prevent buffer overflow on errors longer than 1024 characters #881

Closed
wants to merge 13 commits into from
Closed

Add dynamic buffers in GetDiagRecs and GetErrorFromHandle and prevent buffer overflow on errors longer than 1024 characters #881

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8b4a795
increase error buffer sizes
juliuszsompolski Mar 7, 2021
27cf95b
heap message if stack message is too small
juliuszsompolski Apr 21, 2021
9b4c3d5
error message buffer on heap only
juliuszsompolski Apr 21, 2021
1c07ad0
also do for cursor.cpp GetDiagRecs
juliuszsompolski Apr 21, 2021
4091d91
add long messages to PRINT tests
juliuszsompolski Apr 21, 2021
995aef6
Add BYTE declaration missing from iODBC
gordthompson Mar 10, 2021
14837b3
add pyodbc.pyi stub file
keitherskine Feb 6, 2021
1818e68
executemany type hint correction
keitherskine Feb 7, 2021
053216a
add return statement to GetDiagRecs()
keitherskine Jan 31, 2021
7b8a44f
more rounded PRINT buffer size
keitherskine Feb 5, 2021
499f22b
Merge branch 'master' into long-error-messages
juliuszsompolski Apr 26, 2021
2cd81d8
update tests
juliuszsompolski Apr 26, 2021
d9366a0
WIP: Added PyError_NoMemory
May 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions src/cursor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,12 +574,17 @@ static int GetDiagRecs(Cursor* cur)

ODBCCHAR cSQLState[6]; // five-character SQLSTATE code (plus terminating NULL)
SQLINTEGER iNativeError;
ODBCCHAR cMessageText[10240]; // PRINT statements can be large, hopefully 10K bytes will be enough
SQLSMALLINT iMessageLen = 1023;
ODBCCHAR *cMessageText = (ODBCCHAR*) pyodbc_malloc((iMessageLen + 1) * sizeof(ODBCCHAR));
SQLSMALLINT iTextLength;

SQLRETURN ret;
char sqlstate_ascii[6] = ""; // ASCII version of the SQLState

if (!cMessageText) {
return 0;
}

msg_list = PyList_New(0);
if (!msg_list)
return 0;
Expand All @@ -594,12 +599,29 @@ static int GetDiagRecs(Cursor* cur)
Py_BEGIN_ALLOW_THREADS
ret = SQLGetDiagRecW(
SQL_HANDLE_STMT, cur->hstmt, iRecNumber, (SQLWCHAR*)cSQLState, &iNativeError,
(SQLWCHAR*)cMessageText, (short)(_countof(cMessageText)-1), &iTextLength
(SQLWCHAR*)cMessageText, iMessageLen, &iTextLength
);
Py_END_ALLOW_THREADS
if (!SQL_SUCCEEDED(ret))
break;

// If needed, allocate a bigger error message buffer and retry.
if (iTextLength > iMessageLen - 1) {
iMessageLen = iTextLength + 1;
if (!pyodbc_realloc((BYTE**) &cMessageText, (iMessageLen + 1) * sizeof(ODBCCHAR))) {
pyodbc_free(cMessageText);
return 0;
}
Py_BEGIN_ALLOW_THREADS
ret = SQLGetDiagRecW(
SQL_HANDLE_STMT, cur->hstmt, iRecNumber, (SQLWCHAR*)cSQLState, &iNativeError,
(SQLWCHAR*)cMessageText, iMessageLen, &iTextLength
);
Py_END_ALLOW_THREADS
if (!SQL_SUCCEEDED(ret))
break;
}

cSQLState[5] = 0; // Not always NULL terminated (MS Access)
CopySqlState(cSQLState, sqlstate_ascii);
PyObject* msg_class = PyUnicode_FromFormat("[%s] (%ld)", sqlstate_ascii, (long)iNativeError);
Expand Down Expand Up @@ -635,6 +657,7 @@ static int GetDiagRecs(Cursor* cur)

iRecNumber++;
}
pyodbc_free(cMessageText);

Py_XDECREF(cur->messages);
cur->messages = msg_list; // cur->messages now owns the msg_list reference
Expand Down
38 changes: 31 additions & 7 deletions src/errors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,13 @@ PyObject* GetErrorFromHandle(Connection *conn, const char* szFunction, HDBC hdbc
SQLSMALLINT cchMsg;

ODBCCHAR sqlstateT[6];
ODBCCHAR szMsg[1024];
SQLSMALLINT msgLen = 1023;
ODBCCHAR *szMsg = (ODBCCHAR*) pyodbc_malloc((msgLen + 1) * sizeof(ODBCCHAR));

if (!szMsg) {
PyErr_NoMemory();
return 0;
}

if (hstmt != SQL_NULL_HANDLE)
{
Expand Down Expand Up @@ -251,11 +257,26 @@ PyObject* GetErrorFromHandle(Connection *conn, const char* szFunction, HDBC hdbc

SQLRETURN ret;
Py_BEGIN_ALLOW_THREADS
ret = SQLGetDiagRecW(nHandleType, h, iRecord, (SQLWCHAR*)sqlstateT, &nNativeError, (SQLWCHAR*)szMsg, (short)(_countof(szMsg)-1), &cchMsg);
ret = SQLGetDiagRecW(nHandleType, h, iRecord, (SQLWCHAR*)sqlstateT, &nNativeError, (SQLWCHAR*)szMsg, msgLen, &cchMsg);
Py_END_ALLOW_THREADS
if (!SQL_SUCCEEDED(ret))
break;

// If needed, allocate a bigger error message buffer and retry.
if (cchMsg > msgLen - 1) {
msgLen = cchMsg + 1;
if (!pyodbc_realloc((BYTE**) &szMsg, (msgLen + 1) * sizeof(ODBCCHAR))) {
Comment on lines +267 to +268
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I intentionally did some defensive "double +1" here and in other places. A small cost to avoid potential overflow.

PyErr_NoMemory();
pyodbc_free(szMsg);
return 0;
}
Py_BEGIN_ALLOW_THREADS
ret = SQLGetDiagRecW(nHandleType, h, iRecord, (SQLWCHAR*)sqlstateT, &nNativeError, (SQLWCHAR*)szMsg, msgLen, &cchMsg);
Py_END_ALLOW_THREADS
if (!SQL_SUCCEEDED(ret))
break;
}

// Not always NULL terminated (MS Access)
sqlstateT[5] = 0;

Expand All @@ -272,8 +293,11 @@ PyObject* GetErrorFromHandle(Connection *conn, const char* szFunction, HDBC hdbc
// exception class and append the calling function name.
CopySqlState(sqlstateT, sqlstate);
msg = PyUnicode_FromFormat("[%s] %V (%ld) (%s)", sqlstate, msgStr.Get(), "(null)", (long)nNativeError, szFunction);
if (!msg)
if (!msg) {
PyErr_NoMemory();
pyodbc_free(szMsg);
return 0;
}
}
else
{
Expand All @@ -298,6 +322,9 @@ PyObject* GetErrorFromHandle(Connection *conn, const char* szFunction, HDBC hdbc
#endif
}

// Raw message buffer not needed anymore
pyodbc_free(szMsg);

if (!msg || PyUnicode_GetSize(msg.Get()) == 0)
{
// This only happens using unixODBC. (Haven't tried iODBC yet.) Either the driver or the driver manager is
Expand All @@ -317,14 +344,11 @@ PyObject* GetErrorFromHandle(Connection *conn, const char* szFunction, HDBC hdbc

static bool GetSqlState(HSTMT hstmt, char* szSqlState)
{
SQLCHAR szMsg[300];
SQLSMALLINT cbMsg = (SQLSMALLINT)(_countof(szMsg) - 1);
SQLINTEGER nNative;
SQLSMALLINT cchMsg;
SQLRETURN ret;

Py_BEGIN_ALLOW_THREADS
ret = SQLGetDiagRec(SQL_HANDLE_STMT, hstmt, 1, (SQLCHAR*)szSqlState, &nNative, szMsg, cbMsg, &cchMsg);
ret = SQLGetDiagField(SQL_HANDLE_STMT, hstmt, 1, SQL_DIAG_SQLSTATE, (SQLCHAR*)szSqlState, 5, &cchMsg);
Py_END_ALLOW_THREADS
return SQL_SUCCEEDED(ret);
}
Expand Down