3.52.15 mangles input parameter data when using parameter arrays

matthew-wozniczka commented 2 years ago

I've been debugging some new test failures when using iODBC 3.52.15, and in one scenario, I bind 1 SQL_C_WCHAR parameter, with a paramset size of 4, and a buffer size of 8 (i.e. 2 UTF32 characters). Each paramset contains a single-character, null terminated string, with the length indicators all set to SQL_NTS.

I've found that the new version (before we only used 3.52.8 or older) has some logic for converting parameter data before/after calling into the actual driver (I'm not sure why it's needed for this case, as iODBC itself detects that the driver encoding matches the DM encoding, i.e. UTF32), and while 'converting' (i.e. doing seemingly useless copies) the value of the parameter in the first parameter set, it overwrites the first character of the second parameter set's value with a null-terminator, causing the driver to see that value as an empty string.

I used a watchpoint to see exactly where it was happening:

    frame #0: 0x000000010456d9fc libiodbc.2.dylib`_WCSNCPY(charset=CP_UCS4, dest=0x0000000116c1e540, sour=0x0000000116f046e0, count=2) at unicode.c:2219:10
    frame #1: 0x000000010456da64 libiodbc.2.dylib`DRV_WCSNCPY(conv=0x0000000116c04fac, dest=0x0000000116c1e540, sour=0x0000000116f046e0, count=2) at unicode.c:2243:10
    frame #2: 0x000000010452f09c libiodbc.2.dylib`_ExecConv_W2W(data="1", pInd=0x0000000116c1e520, size=8, conv=0x0000000116c04fac, bOutput=NO) at execute.c:366:15
    frame #3: 0x000000010452ee24 libiodbc.2.dylib`_ConvParam(pstmt=0x0000000116c05040, pparm=0x0000000117808200, row=0, bOutput=NO, conv=0x0000000116c04fac, unicode_driver=1) at execute.c:448:5
    frame #4: 0x000000010452c280 libiodbc.2.dylib`_SQLExecute_ConvParams(hstmt=0x0000000116c05040, bOutput=NO) at execute.c:983:17
    frame #5: 0x000000010452acfc libiodbc.2.dylib`SQLExecute_Internal(hstmt=0x0000000116c05040) at execute.c:1072:13
    frame #6: 0x000000010452ab4c libiodbc.2.dylib`SQLExecute(hstmt=0x0000000116c05040) at execute.c:1166:13

https://github.com/openlink/iODBC/blob/79c7f572a7b5c4123ec3cc1dd29df1af61a3405f/iodbcinst/unicode.c#L2213 this logic is wrong, for my case, count==2, and the buffer contains 0x31 0x00 0x00 0x00 0x00 0x00 0x00 0x00, so on the second iteration of the loop, it exits, as it copied a 'null character'. But u4dst has already been incremented twice, even though len==1, so below it tries to write another null terminator, overwriting the first character of the next parameter set's value. This causes our driver to see the second parameter set's value as an empty string, instead of what it was actually set to by the application.

matthew-wozniczka commented 2 years ago

I think this could lead to a buffer overflow, even if not using parameter arrays