FreeTDS Development Group

["Lowden, James K" <LowdenJK AT bernstein.com>]

> From: L J Bayuk [mailto:lbayuk AT mindspring.com]
> Sent: July 7, 2002 9:30 PM
> 
> When I select from a TEXT field, the last character of
> the value is lost. (CHAR and VARCHAR are OK).  This only happens when
> using a recent FreeTDS "current" CVS snapshot, not when using 
> FreeTDS-0.53.  It works fine in 0.53.
> 
> I think the problem is an off-by-1 error here in 
> src/tds/convert.c in the
> function tds_convert_text():
>          cplen = srclen > destlen ? destlen : srclen;
>          memcpy(dest, src, cplen);
> *        dest[cplen-1] = '\0';
>          return strlen(dest);

Brian, what do think about this and the proposed patch?  The story in cvs is
an unhappy one.  I looked at a diff for src/tds/convert.c between version
1.12 and 1.13; there's a little tug of war over whether and when cplen
should be decremented.  We seem to be oscillating between truncating the
data and overflowing the buffer.  :/  My ten foot pole says not to touch it.


--jkl

> I think that (*) line should be: dest[cplen] = '\0';
> (that's how it was at 0.53) but it seems to me this could 
> overflow the dest
> buffer.  I browsed the CVS on SourceForge and found that this 
> was changed
> in rev 1.13 of that file on May 25, 2002 by Victor K. but it 
> isn't clear
> why. I would like to propose the patch below. It changes it back, and
> includes another change to prevent dest buffer overflow, and 
> a third change
> in how to get the return value. These make sense to me, and 
> seem to work,
> but I'm new with this so dissenting opinions would be welcome...
> 
> *** convert.c.bak     Tue Jun 25 21:44:27 2002
> --- convert.c Sat Jul  6 10:41:43 2002
> ***************
> *** 122,131 ****
>      switch(desttype) {
>         case SYBTEXT:
>         case SYBCHAR:
> !      cplen = srclen > destlen ? destlen : srclen;
>            memcpy(dest, src, cplen);
> !          dest[cplen-1] = '\0';
> !          return strlen(dest);
>         case SYBBINARY:
>        cplen = srclen > destlen ? destlen : srclen;
>            memcpy(dest, src, cplen);
> --- 122,131 ----
>      switch(desttype) {
>         case SYBTEXT:
>         case SYBCHAR:
> !      cplen = srclen >= destlen ? destlen-1 : srclen;
>            memcpy(dest, src, cplen);
> !          dest[cplen] = '\0';
> !          return cplen;
>         case SYBBINARY:
>        cplen = srclen > destlen ? destlen : srclen;
>            memcpy(dest, src, cplen);