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

Buffer overflow in cursor create_name_map #931

Merged
merged 13 commits into from
Aug 16, 2021
Merged

Buffer overflow in cursor create_name_map #931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b8b005c
column name buffer overflow
juliuszsompolski Jul 14, 2021
da393c6
fix
juliuszsompolski Jul 14, 2021
5574c7e
add tests - lets see if these databases actually handle such long col…
juliuszsompolski Jul 14, 2021
2b738f8
remove unsupported tests
juliuszsompolski Jul 14, 2021
c72289a
fix
juliuszsompolski Jul 14, 2021
033a9e4
remove Access, Informix, SQL DW tests
juliuszsompolski Jul 15, 2021
baf350c
whitespace
juliuszsompolski Jul 15, 2021
5794b9b
whitespace cd
juliuszsompolski Jul 15, 2021
b550078
remove sqllite tests as well
juliuszsompolski Jul 15, 2021
89d9a2d
check if maybe long query output columns work
juliuszsompolski Jul 15, 2021
27278c4
Revert "check if maybe long query output columns work"
juliuszsompolski Jul 15, 2021
12095f3
add spark tests
juliuszsompolski Jul 15, 2021
24166ff
revert nameLen change
juliuszsompolski Jul 15, 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
20 changes: 17 additions & 3 deletions src/cursor.cpp
View file
Open in desktop
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