Skip to content

Commit 0f8f90f

Browse files
committed
Fix ConvertingString heap overflow without causing stack smash
Commit 832d8e7 changed lengthString = length/sizeof(wchar_t) to length/sizeof(SQLWCHAR) to stop a 1-byte heap overflow in the internal byteString when OdbcError::sqlGetDiagRec strcpy's a 6-byte SQL state into the 5-byte buffer allocated on Linux (wchar_t=4). That fix is wrong in the opposite direction: lengthString is also the count passed to mbstowcs((wchar_t*)unicodeString, byteString, lengthString) in the destructor, which writes lengthString * sizeof(wchar_t) bytes into the caller's SQLWCHAR buffer. With lengthString=6 and a 12-byte caller buffer, mbstowcs overruns by 12 bytes and smashes the caller's stack (reproducible without ASAN). Revert line 88 to sizeof(wchar_t) to restore the safe mbstowcs bound, and instead floor the internal byteString allocation at 8 bytes so the strcpy no longer overflows. This is a targeted fix that keeps both the ASAN job and normal runs green until the ConvertingString / mbstowcs rewrite lands (issue #287 Tier 9.1).
1 parent 8258a27 commit 0f8f90f

1 file changed

Lines changed: 11 additions & 3 deletions

File tree

MainUnicode.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class ConvertingString
8585
if ( length == SQL_NTS )
8686
lengthString = 0;
8787
else if ( retCountOfBytes )
88-
lengthString = length / sizeof(SQLWCHAR);
88+
lengthString = length / sizeof(wchar_t);
8989
else
9090
lengthString = length;
9191
}
@@ -219,8 +219,16 @@ class ConvertingString
219219
case BYTESCHARS:
220220
if ( lengthString )
221221
{
222-
byteString = new SQLCHAR[ lengthString + 2 ];
223-
memset(byteString, 0, lengthString + 2);
222+
// Floor the internal buffer at 8 bytes so that callers which pass a
223+
// small SQLWCHAR output buffer (e.g. SQLGetDiagRecW with a 12-byte
224+
// SQL state, yielding lengthString=3 on Linux where sizeof(wchar_t)=4)
225+
// still have room for the 6-byte SQL state ("HY000\0") that
226+
// OdbcError::sqlGetDiagRec strcpy's into this buffer. Keeping
227+
// lengthString itself unchanged preserves the mbstowcs writeback
228+
// bound and avoids smashing the caller's stack buffer.
229+
const size_t bufSize = (lengthString + 2 < 8) ? 8 : (size_t)lengthString + 2;
230+
byteString = new SQLCHAR[ bufSize ];
231+
memset(byteString, 0, bufSize);
224232
}
225233
else
226234
byteString = NULL;

0 commit comments

Comments
 (0)