Skip to Content.
Sympa Menu

freetds - Re: [freetds] Strange assert

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: "James K. Lowden" <jklowden AT schemamania.org>
  • To: FreeTDS Development Group <freetds AT lists.ibiblio.org>
  • Subject: Re: [freetds] Strange assert
  • Date: Sat, 15 Mar 2003 10:14:18 -0500

On 15 Mar 2003 12:23:18 +0100, Frediano Ziglio <freddyz77 AT tin.it> wrote:
> I found a new, strange assert in dblib.c (dbconvert)
>
> case -1: /* rtrim and null terminate */
> for (i = len - 1; i >= 0 && dres.c[i] == ' '; --i) {
> len = i;
> }
> assert(dres.c[len] == ' '); /* if last position
> is not a
> blank, no
> room to null terminate */
> memcpy(dest, dres.c, len);
> dest[len] = '\0';
> ret = len;
> break;
>
> Why should dres.c contain space for terminator if copied and terminated
> in copied buffer?
> Also dres.c is not terminated, so this assert cause a core in some
> condition (it's a read buffer overflow)...

The assert is wrong, and so is the code, still.

We are going to write dest[len], but "len" is the count of bytes, returned
from tds_convert. "len" can be decreased by the for() loop, but that loop
won't execute if dres.c[len-1] is not a blank. There's no assurance that
"len" fits in dest.

The assert should have been:

assert(len < destlen);

What do you say?

--jkl




Archive powered by MHonArc 2.6.24.

Top of Page