Microsoft ODBC bug

[Arran Cudbard-Bell]

> On 2 Jul 2019, at 13:16, Arran Cudbard-Bell <a.cudbardb at freeradius.org> wrote:
> 
> 
> 
>> On 2 Jul 2019, at 12:21, Dom Latter <freeradius-users at latter.org> wrote:
>> 
>> 
>> 
>> On 02/07/2019 16:10, Alan DeKok wrote:
>>> On Jul 2, 2019, at 12:18 PM, Dom Latter <freeradius-users at latter.org>
>>> wrote:
>>>> And again, with PHP, "SELECT 123456789" works fine but a large number gets an error from the ODBC driver.
>>>> <snip>
>>> That's all well and good, but what should *we* be doing differently?
>> 
>> Well, if I knew that...
>> 
>>> We're not Microsoft experts, or experts in ODBC.  The ODBC layer was
>>> contributed by someone years ago, and we've maintained it since then.
>>> It mostly works, but new features require people who can delve into
>>> it and fix things.
>> 
>> And that is what I am trying to do.  I am looking at the following
>> in rlm_sql_unixodbc.c
>> 
>> /* Executing query */
>> {
>> 	SQLCHAR *odbc_query;
>> 
>> 	memcpy(&odbc_query, &query, sizeof(odbc_query));
>> 	err_handle = SQLExecDirect(conn->stmt, odbc_query, strlen(query));
>> }
>> 
>> Is that a safe memcpy?  It's a long time since I programmed in C...
> 
> It's likely copying a pointer from query to odbc_query to defeat const checks, because SQLExecDirect should likely take a const (read only) query string pointer, and doesn't.
> 
> So yes... It's fine.

If you wanted to double check what was being passed, either break at that line in a debugger, or add:

ERROR("Query executed: %s", (char *)odbc_query);

somewhere after the memcpy, and it'll print out the query string in red.

-Arran

Arran Cudbard-Bell <a.cudbardb at freeradius.org>
FreeRADIUS Development Team

FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2