Skip to content

Commit

Permalink
ODBC-321 The fix and the testcase
Browse files Browse the repository at this point in the history
atm it's not clear, if the actual reporter's problem has been fixed.
But from my point of view - it is.
The problem was that buffer length in bytes provided in SQLBindCol, was
treated as char lenght during the conversion to utf16. That could
obviously cause buffer overflow.
  • Loading branch information
lawrinn committed Aug 9, 2021
1 parent 61a7763 commit 18f9675
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 4 deletions.
2 changes: 1 addition & 1 deletion ma_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ SQLRETURN MADB_DbcGetAttr(MADB_Dbc *Dbc, SQLINTEGER Attribute, SQLPOINTER ValueP
if (!SQL_SUCCEEDED(ret) && Dbc->CatalogName)
{
MADB_CLEAR_ERROR(&Dbc->Error);
StrLen= (SQLSMALLINT)MADB_SetString(isWChar ? &Dbc->Charset : 0, ValuePtr, BufferLength,
StrLen= (SQLSMALLINT)MADB_SetString(isWChar ? &Dbc->Charset : 0, ValuePtr, BUFFER_CHAR_LEN(BufferLength, isWChar),
Dbc->CatalogName, strlen(Dbc->CatalogName), &Dbc->Error);
ret= SQL_SUCCESS;
}
Expand Down
8 changes: 8 additions & 0 deletions ma_platform_posix.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ int MADB_ConvertAnsi2Unicode(Client_Charset *cc, const char *AnsiString, SQLLEN
if (RequiredLength > UnicodeLength)
{
Tmp= (SQLWCHAR *)malloc(RequiredLength * sizeof(SQLWCHAR));
if (Tmp == NULL)
{
if (Error)
{
MADB_SetError(Error, MADB_ERR_HY001, NULL, 0);
}
return 1;
}
}
else
{
Expand Down
27 changes: 25 additions & 2 deletions ma_platform_win32.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,15 @@ char *MADB_ConvertFromWChar(const SQLWCHAR *Wstr, SQLINTEGER WstrCharLen, SQLULE
}
/* }}} */

/* Required Length without or with TN(if IsNull is TRUE, or AnsiLength == -1 or SQL_NTS) is put to LenghtIndicator*/
/* {{{ MADB_ConvertAnsi2Unicode
@AnsiLength[in] - number of bytes to copy, negative if AnsiString is Null terminated and the terminating blank has to be copied
@UnicodeLength[in] - size of output buffer in chars, that effectively mean in SQLWCHAR units
@LengthIndicator[out] - number of available characters not counting terminating null(unless it was included in AnsiLength, and IsNull
is FALSE)
@IsNull[in] - whether to copy terminating blank. The value has to be 1 or 0(TRUE/FALSE)
If AnsiString is negative, its value is neglected(null is copied)
@returns 1 in case of error, 0 otherwise */
/* Required Length without or with TN(if IsNull is TRUE, or AnsiLength == -1 or SQL_NTS) is put to LenghtIndicator */
int MADB_ConvertAnsi2Unicode(Client_Charset *cc, const char *AnsiString, SQLLEN AnsiLength,
SQLWCHAR *UnicodeString, SQLLEN UnicodeLength,
SQLLEN *LengthIndicator, BOOL IsNull, MADB_Error *Error)
Expand Down Expand Up @@ -197,7 +205,22 @@ int MADB_ConvertAnsi2Unicode(Client_Charset *cc, const char *AnsiString, SQLLEN
return 0;

if (RequiredLength > UnicodeLength)
Tmp= (SQLWCHAR *)malloc(RequiredLength * sizeof(SQLWCHAR));
{
Tmp= (SQLWCHAR*)malloc(RequiredLength * sizeof(SQLWCHAR));
if (Tmp == NULL)
{
if (Error)
{
MADB_SetError(Error, MADB_ERR_HY001, NULL, 0);
}
return 1;
}
}
else
{
/* Otherwise the size of buffer is, as passed to the this function, UnicodeLength */
RequiredLength= UnicodeLength;
}

RequiredLength= MultiByteToWideChar(cc->CodePage, 0, AnsiString, IsNull ? -1 : (int)AnsiLength, Tmp, (int)RequiredLength);
if (RequiredLength < 1)
Expand Down
8 changes: 7 additions & 1 deletion ma_statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -1886,8 +1886,14 @@ SQLRETURN MADB_FixFetchedValues(MADB_Stmt *Stmt, int RowNumber, MYSQL_ROW_OFFSET
break;
case SQL_C_WCHAR:
{
SQLLEN CharLen= MADB_SetString(&Stmt->Connection->Charset, DataPtr, ArdRec->OctetLength, (char *)Stmt->result[i].buffer,
SQLLEN CharLen= MADB_SetString(&Stmt->Connection->Charset, DataPtr, ArdRec->OctetLength/sizeof(SQLWCHAR), (char *)Stmt->result[i].buffer,
*Stmt->stmt->bind[i].length, &Stmt->Error);

/* If returned len is 0 while source len is not - taking it as error occurred */
if ((CharLen == 0 || CharLen > (ArdRec->OctetLength / sizeof(SQLWCHAR))) && *Stmt->stmt->bind[i].length != 0 && *(char*)Stmt->result[i].buffer != '\0' && Stmt->Error.ReturnValue != SQL_SUCCESS)
{
CALC_ALL_FLDS_RC(rc, Stmt->Error.ReturnValue);
}
/* Not quite right */
*LengthPtr= CharLen * sizeof(SQLWCHAR);
}
Expand Down
23 changes: 23 additions & 0 deletions test/unicode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1595,6 +1595,28 @@ ODBC_TEST(t_odbc253)
return OK;
}

/* ODBC-321 atm it appears that crash occurs because bind length given in bytes, treated as char lenght during result conversion.
The test tries provides shorter buffer, than needed. But bytes length is > then required length in chars */
ODBC_TEST(t_odbc321)
{
SQLWCHAR a[2];
SQLWCHAR a_ref[] = { 'a', 0 };
SQLLEN lenPtr;

OK_SIMPLE_STMTW(Stmt, CW("SELECT 'abc'"));

CHECK_STMT_RC(Stmt, SQLBindCol(Stmt, 1, SQL_C_WCHAR, a, sizeof(a), &lenPtr));

EXPECT_STMT(Stmt, SQLFetch(Stmt), SQL_SUCCESS_WITH_INFO);

IS_WSTR(a, a_ref, sizeof(a_ref) / sizeof(SQLWCHAR));
is_num(lenPtr, 3/*'abc'*/*sizeof(SQLWCHAR));

CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));

return OK;
}


MA_ODBC_TESTS my_tests[]=
{
Expand Down Expand Up @@ -1627,6 +1649,7 @@ MA_ODBC_TESTS my_tests[]=
{t_odbc72, "odbc72_surrogate_pairs", NORMAL},
{t_odbc203, "t_odbc203", NORMAL},
{t_odbc253, "t_odbc253_empty_str_crash", NORMAL},
{t_odbc321, "t_odbc321_short_buffer", NORMAL},
{NULL, NULL}
};

Expand Down

0 comments on commit 18f9675

Please sign in to comment.