Signed / unsigned comparison issue in Data\ODBC\include\Poco\Data\ODBC\Extractor.h

[shandyba]

Abovementioned file contains the following assertion check inside function:

template<typename T>
bool extractManualImpl(std::size_t pos, T& val, SQLSMALLINT cType):

//for fixed-length data, buffer must be large enough
//otherwise, driver may write past the end
poco_assert_dbg (len <= sizeof(T));

Unfortunately len may very well be negative (e.g. SQL_NULL_DATA predefined value).
In this case signed / unsigned comparison between signed len and unsigned sizeof() result has unexpected behavior (false assertion is raised).

This is reproduced in VS 2012 by simplistic sample as well:

if (-1 <= sizeof(int))
{
     std::cout << "good" << std::endl;
}
else
{
    std::cout << "bad" << std::endl;
}

Casting sizeof() result in place to signed value fixes the issue.

[aleks-f]

Please specify which version you are using. Here's current develop code (i.e. 1.5.1 relase):

if (isNullLengthIndicator(_lengths[pos])) 
    return false;
else 
{
    //for fixed-length data, buffer must be large enough
    //otherwise, driver may write past the end
    poco_assert_dbg (_lengths[pos] <= sizeof(T));
    val = value;
}

[shandyba]

Hi Aleks,

thanks for your response!

I'm using current stable release, e.g. 1.4.6, and, therefore, my report refers to that version, sorry for not mentioning it in my original post.

Development code (1.5.1) that you pointed me out to has reworked that function and covers the case of SQL_NULL_DATA by performing a isNullLengthIndicator check before the comparison. At the same time as there still takes place a signed / unsigned comparison, other possible negative values of _lengths[pos], which is a signed value, either int64 or long (e.g. SQL_DATA_AT_EXEC), will result in false assertion. I'm not sure if SQL_DATA_AT_EXEC may be achieved at that exact point of code in semantically correct way, but if so (or if any other negative "reserved" length value is returned), the issue will still take place.

Dmitry.

[aleks-f]

The only two values (besides the length itself) that can be returned ( see http://msdn.microsoft.com/en-us/library/windows/desktop/ms715441(v=vs.85).aspx ) are SQL_NULL_DATA and SQL_NO_TOTAL. Since SQL_NO_TOTAL only applies to character and binary data, it is handled in specializations of extractManualImpl:

if (SQL_NO_TOTAL == len)//unknown length, throw
    throw UnknownDataLengthException("Could not determine returned data length.");

if (isNullLengthIndicator(len))
{ ...

EDIT: 2/16/03 1705UTC edited code snippet to reflect both values being checked

[shandyba]

Yes, I agree.

I was mislead by generic topic on length / indicator values (http://msdn.microsoft.com/en-us/library/windows/desktop/ms713532(v=vs.85).aspx) which applies not only to SQLGetData but to other SQL* functions as well.

Therefore the issue is relevant only in current (1.4.6) version and looks to be addressed in 1.5.x.

Thanks,
Dmitry.

[aleks-f]

@obiltschnig : any plans to address this in 1.4.x ? if not, or it was done, let's close it