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.

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

Buffer overflow in cursor create_name_map #931

Merged
merged 13 commits into from
Aug 16, 2021
20 changes: 17 additions & 3 deletions src/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ static bool create_name_map(Cursor* cur, SQLSMALLINT field_count, bool lower)

bool success = false;
PyObject *desc = 0, *colmap = 0, *colinfo = 0, *type = 0, *index = 0, *nullable_obj=0;
SQLSMALLINT nameLen = 300;
ODBCCHAR *szName = NULL;
SQLRETURN ret;

I(cur->hstmt != SQL_NULL_HANDLE && cur->colinfos != 0);
Expand All @@ -148,20 +150,21 @@ static bool create_name_map(Cursor* cur, SQLSMALLINT field_count, bool lower)

desc = PyTuple_New((Py_ssize_t)field_count);
colmap = PyDict_New();
if (!desc || !colmap)
szName = (ODBCCHAR*) pyodbc_malloc((nameLen + 1) * sizeof(ODBCCHAR));
if (!desc || !colmap || !szName)
goto done;

for (int i = 0; i < field_count; i++)
{
ODBCCHAR szName[300];
SQLSMALLINT cchName;
SQLSMALLINT nDataType;
SQLULEN nColSize; // precision
SQLSMALLINT cDecimalDigits; // scale
SQLSMALLINT nullable;

retry:
Py_BEGIN_ALLOW_THREADS
ret = SQLDescribeColW(cur->hstmt, (SQLUSMALLINT)(i + 1), (SQLWCHAR*)szName, _countof(szName), &cchName, &nDataType, &nColSize, &cDecimalDigits, &nullable);
ret = SQLDescribeColW(cur->hstmt, (SQLUSMALLINT)(i + 1), (SQLWCHAR*)szName, nameLen, &cchName, &nDataType, &nColSize, &cDecimalDigits, &nullable);
Py_END_ALLOW_THREADS

if (cur->cnxn->hdbc == SQL_NULL_HANDLE)
Expand All @@ -177,6 +180,16 @@ static bool create_name_map(Cursor* cur, SQLSMALLINT field_count, bool lower)
goto done;
}

// If needed, allocate a bigger column name message buffer and retry.
if (cchName > nameLen - 1) {
nameLen = cchName + 1;
if (!pyodbc_realloc((BYTE**) &szName, (nameLen + 1) * sizeof(ODBCCHAR))) {
Comment on lines +185 to +186
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like in my previous PR, I prefer to do some defensive double +1 here to rather have a too big buffer, and space for an extra character (like a termination character) than a too small one.

PyErr_NoMemory();
goto done;
}
goto retry;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think goto is justified here to not repeat that logic...

}

const TextEnc& enc = cur->cnxn->metadata_enc;

// HACK: I don't know the exact issue, but iODBC + Teradata results in either UCS4 data
Expand Down Expand Up @@ -289,6 +302,7 @@ static bool create_name_map(Cursor* cur, SQLSMALLINT field_count, bool lower)
Py_XDECREF(colmap);
Py_XDECREF(index);
Py_XDECREF(colinfo);
pyodbc_free(szName);

return success;
}
Expand Down
Loading