DevHeads.net

Bug in is_setting_search_path

Hello,

I have found a bug in the is_setting_search_path function (connection.c line 1597).

The search loop is currently:

for (; *q; q++)
{
if (IS_NOT_SPACE(*q))
{
if (strnicmp(q, "search_path", 11) == 0)
return TRUE;
q++;
while (IS_NOT_SPACE(*q))
q++;
}
}

The inner while(IS_NOT_SPACE(*q)) loop will terminate if a null is reached. The loop variable will then be further incremented by the "for" loop, to point beyond the null terminator, and so the loop will continue, until by chance two nulls are encountered. If two nulls are not found, then eventually the loop will reach the end of the memory page, and cause an access violation. Note that if the string "search_path" exists in memory beyond the end of the statement, a false positive results from this function.

The fix is to remove the increment from the "for" loop, and move it instead to the false path of the "if":

for (; *q;)
{
if (IS_NOT_SPACE(*q))
{
if (strnicmp(q, "search_path", 11) == 0)
return TRUE;
q++;
while (IS_NOT_SPACE(*q))
q++;
}
else
q++
}

This issue has been causing occasional access violations in our code (when calling SET LC_TIME=''). We are currently testing with a fixed version, which is giving no other problems so far.

Please advise me if there is some other way I should submit this change for review and inclusion.

Thankyou

Grant Shirreffs
Principal Developer
StayinFront Inc

Comments

Re: Bug in is_setting_search_path

By Inoue, Hiroshi at 01/11/2018 - 17:47

Hi Grant,

Thanks for the report.
I would commit the fix.

regards,
Hiroshi Inoue

On 2018/01/11 9:17, Grant Shirreffs wrote: