Skip to Content.
Sympa Menu

freetds - Re: Buffer Overflow ?

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: Steve Langasek <vorlon AT netexpress.net>
  • To: TDS Development Group <freetds AT franklin.oit.unc.edu>
  • Subject: Re: Buffer Overflow ?
  • Date: Thu, 25 Jan 2001 14:45:45 -0600 (CST)


Hi Brian,

On Wed, 24 Jan 2001, Brian Bruns wrote:

> Technically it is defined by the API as length of dest, I don't have the
> doc in front of me but I believe it is something like 'size of buffer
> into which the data will be copied' (remember that tds_convert is called
> by dbconvert, ct_convert and implicitly by binding a to a different type).

> So there is an off by one error, and I'll apply the patch.

At the bottom is a patch that addresses all of the off-by-one errors I found
in tds/convert.c. In addition, it ensures that all SYBTEXT and SYBCHAR
targets are null-terminated strings, which if I understand correctly is the
proper behavior.

I also noticed these lines during the process:

TDS_INT tds_convert_money(int srctype,unsigned char *src,
int desttype,unsigned char *dest,TDS_INT destlen)
{
long high;
unsigned long low;
double dmoney;

...

case SYBFLT8:
/* Used memcpy to avoid alignment/bus errors */
memcpy(&high, src, 4);
memcpy(&low, src+4, 4);
dmoney = (double)high * 65536 * 65536 + (double)low;
dmoney = dmoney / 10000;
*(double *)dest = dmoney;
break;

I don't have a patch for this yet, but just as a heads-up, this behavior is
not endian-clean on machines with a 64-bit word. Since a long int is 8 bytes
long on alpha, ppc, etc., these memcpy's only write to the first 4 bytes of
the long... and those first 4 bytes aren't guaranteed to be the low bytes
we're looking for.

If anyone here is better at writing endian-clean code than I am, you're
welcome to beat me to patching this. :)

Regards,
Steve Langasek
postmodern programmer

Index: src/tds/convert.c
===================================================================
RCS file: /Repository/freetds/src/tds/convert.c,v
retrieving revision 1.45
diff -u -w -r1.45 convert.c
--- convert.c 2001/01/21 20:47:11 1.45
+++ convert.c 2001/01/25 20:21:03
@@ -97,13 +97,13 @@
case SYBTEXT:
cplen = srclen > destlen ? destlen : srclen;
memcpy(dest, src, cplen);
+ dest[cplen-1] = '\0';
return cplen;
case SYBCHAR:
- cplen = srclen;
- if (destlen<cplen) cplen = destlen;
+ cplen = srclen > destlen ? destlen : srclen;
if (cplen>255) cplen=255;
memcpy(dest, src, cplen);
- dest[cplen]='\0';
+ dest[cplen-1]='\0';
return cplen;
}
return 0;
@@ -126,7 +126,7 @@
* XXX Case for -1 in switch statement because upper levels don't
* have a way to bind to wide-character types.
*/
- TDS_UINT i, cplen, char_limit = 255;
+ TDS_UINT i, cplen, char_limit = 256;
utf16_t* wsrc = (utf16_t*)src;
utf16_t* wdest = (utf16_t*)dest;
assert(sizeof(utf16_t) == 2);
@@ -155,10 +155,7 @@
cplen = srclen > destlen ? destlen : srclen;
for (i = 0; i < cplen; ++i)
dest[i] = (unsigned char)wsrc[i];
- if (destlen < srclen)
- dest[destlen - 1] = 0;
- else
- dest[cplen] = 0;
+ dest[cplen-1] = 0;
return strlen(dest);
}
return 0;
@@ -176,7 +173,7 @@
case SYBCHAR:
case SYBVARCHAR:
/* FIX ME */
- if (destlen < 2) return 0;
+ if (destlen < 3) return 0;

d=0;
dest[d++]='0';
@@ -588,9 +585,10 @@
case SYBVARCHAR:
/* A real type has a 7 digit precision plus a two digit exponent
* "-d.dddddde+dd"
- * so it should have >=13 bytes allocated, right?
+ * so it should have >=14 bytes allocated (one for the terminating
+ * NULL), right?
*/
- if (destlen >= 0 && destlen < 13) return TDS_FAIL;
+ if (destlen >= 0 && destlen < 14) return TDS_FAIL;
sprintf(dest,"%.7g", the_value);
return strlen(dest);
case SYBFLT8:
@@ -614,9 +612,10 @@
case SYBVARCHAR:
/* A flt8 type has a 15 digit precision plus a three digit exponent
* "-d.dddddddddddddde+ddd"
- * so it should have >=22 bytes allocated, right?
+ * so it should have >=23 bytes allocated (one for the terminating
+ * NULL), right?
*/
- if (destlen >= 0 && destlen < 22) return TDS_FAIL;
+ if (destlen >= 0 && destlen < 23) return TDS_FAIL;
sprintf(dest,"%.15g", the_value);
return strlen(dest);
case SYBREAL:






Archive powered by MHonArc 2.6.24.

Top of Page