FreeTDS Development Group

["Matthew A. Davis" <azami AT speakeasy.net>]

We discovered this when running a query that ends with a quoted string, 
followed by a different query that also ends with a quoted string but happens 
to be exactly 1 character longer than the first one.

Here's the function:

static const char *
tds_skip_quoted_ucs2le(const char *s, const char *end)
{
        const char *p = s;
        char quote = (*s == '[') ? ']' : *s;

        assert(s[1] == 0 && s < end && (end - s) % 2 == 0);

        for (; (p += 2) != end;) {
                if (p[0] == quote && !p[1]) {
                        p += 2;
                        if (p[0] != quote || p[1])
                                return p;
                }
        }
        return p;
}

The assert is firing because s < end does not hold; in fact, s is quite a 
ways past end.

In this function, s is not null terminated; it relies on end to determine the 
length of the string.  end itself points to the first character past the end 
of the string, which actually contains arbitrary garbage.

Now, consider what happens if the last valid character in the string is the 
ending quote we're looking for, and the following garbage character _happens_ 
to be another quote.  The code inside the if clause will increment p to point 
to the second quote and interpret it as an escaped quote - meaning
it needs to continue looking.  Then as we loop, p gets incremented again - 
past the end - and the loop continues to run through whatever garbage is in 
memory until it finds another single quote on an even byte, followed by a 0 
byte.

(The calling code is then continues to look, and if it finds another quote, 
it'll go back into this function, and the assert will trigger.)

I suggest that changing the if clause as follows will correct the bug:

                if (p[0] == quote && !p[1]) {
                        p += 2;
                        if (p[0] != quote || p[1] || p == end)
                                return p;
                }

-- 
Matthew A. Davis  ( mailto:azami AT speakeasy.net )
http://www.speakeasy.net/~azami

 "It's not always popular upholding the Constitution,
  but it's always the right thing to do."
                     -Senator Patrick J. Leahy (D-Vt)