__STRINGTOLD_L expects a pointer to a `_LDOUBLE` as its first argument, which we internally declare as `MSVCRT__LDOUBLE`. Problem is, `_LDOUBLE` is 10 bytes long, whereas `MSVCRT__LDOUBLE` is 12 bytes long (ULONG * 3). Writing into the third ULONG is technically a buffer overflow.
Declare `MSVCRT__LDOUBLE` instead as 2 ULONGs plus a USHORT.
-- v2: msvcrt: Use _LDOUBLE type in exported functions.
From: Yuxuan Shui yshui@codeweavers.com
It fixes an ASan report due to write access to structure padding. --- dlls/msvcrt/msvcrt.h | 2 - dlls/msvcrt/string.c | 94 ++++++++++++++++++++++---------------------- 2 files changed, 47 insertions(+), 49 deletions(-)
diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h index 77170fff919..47cf70f1cc9 100644 --- a/dlls/msvcrt/msvcrt.h +++ b/dlls/msvcrt/msvcrt.h @@ -79,8 +79,6 @@ void __cdecl terminate(void);
typedef void (__cdecl *MSVCRT_security_error_handler)(int, void *);
-typedef struct {ULONG x80[3];} MSVCRT__LDOUBLE; /* Intel 80 bit FP format has sizeof() 12 */ - typedef struct __lc_time_data { union { const char *str[43]; diff --git a/dlls/msvcrt/string.c b/dlls/msvcrt/string.c index 90538533d95..6b4ffbf45b5 100644 --- a/dlls/msvcrt/string.c +++ b/dlls/msvcrt/string.c @@ -36,6 +36,12 @@
WINE_DEFAULT_DEBUG_CHANNEL(msvcrt);
+struct MSVCRT__LDOUBLE +{ + ULONGLONG m; + unsigned short exp; +}; + /********************************************************************* * _mbsdup (MSVCRT.@) * _strdup (MSVCRT.@) @@ -474,25 +480,23 @@ int fpnum_double(struct fpnum *fp, double *d)
#define LDBL_EXP_BITS 15 #define LDBL_MANT_BITS 64 -int fpnum_ldouble(struct fpnum *fp, MSVCRT__LDOUBLE *d) +int fpnum_ldouble(struct fpnum *fp, struct MSVCRT__LDOUBLE *d) { if (fp->mod == FP_VAL_INFINITY) { - d->x80[0] = 0; - d->x80[1] = 0x80000000; - d->x80[2] = (1 << LDBL_EXP_BITS) - 1; + d->m = 0x8000000000000000ull; + d->exp = (1 << LDBL_EXP_BITS) - 1; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return 0; }
if (fp->mod == FP_VAL_NAN) { - d->x80[0] = ~0; - d->x80[1] = ~0; - d->x80[2] = (1 << LDBL_EXP_BITS) - 1; + d->m = ~0ull; + d->exp = (1 << LDBL_EXP_BITS) - 1; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return 0; }
@@ -500,31 +504,28 @@ int fpnum_ldouble(struct fpnum *fp, MSVCRT__LDOUBLE *d) fp->m, fp->exp, fp->mod); if (!fp->m) { - d->x80[0] = 0; - d->x80[1] = 0; - d->x80[2] = 0; + d->m = 0; + d->exp = 0; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return 0; }
/* make sure that we don't overflow modifying exponent */ if (fp->exp > 1<<LDBL_EXP_BITS) { - d->x80[0] = 0; - d->x80[1] = 0x80000000; - d->x80[2] = (1 << LDBL_EXP_BITS) - 1; + d->m = 0x8000000000000000ull; + d->exp = (1 << LDBL_EXP_BITS) - 1; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return ERANGE; } if (fp->exp < -(1<<LDBL_EXP_BITS)) { - d->x80[0] = 0; - d->x80[1] = 0; - d->x80[2] = 0; + d->m = 0; + d->exp = 0; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return ERANGE; } fp->exp += LDBL_MANT_BITS - 1; @@ -573,28 +574,25 @@ int fpnum_ldouble(struct fpnum *fp, MSVCRT__LDOUBLE *d)
if (fp->exp >= (1<<LDBL_EXP_BITS)-1) { - d->x80[0] = 0; - d->x80[1] = 0x80000000; - d->x80[2] = (1 << LDBL_EXP_BITS) - 1; + d->m = 0x8000000000000000ull; + d->exp = (1 << LDBL_EXP_BITS) - 1; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return ERANGE; } if (!fp->m || fp->exp < 0) { - d->x80[0] = 0; - d->x80[1] = 0; - d->x80[2] = 0; + d->m = 0; + d->exp = 0; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return ERANGE; }
- d->x80[0] = fp->m; - d->x80[1] = fp->m >> 32; - d->x80[2] = fp->exp; + d->m = fp->m; + d->exp = fp->exp; if (fp->sign == -1) - d->x80[2] |= 1 << LDBL_EXP_BITS; + d->exp |= 1 << LDBL_EXP_BITS; return 0; }
@@ -1501,9 +1499,10 @@ size_t CDECL strxfrm( char *dest, const char *src, size_t len ) /******************************************************************** * __STRINGTOLD_L (MSVCR80.@) */ -int CDECL __STRINGTOLD_L( MSVCRT__LDOUBLE *value, char **endptr, +int CDECL __STRINGTOLD_L( _LDOUBLE *value, char **endptr, const char *str, int flags, _locale_t locale ) { + struct MSVCRT__LDOUBLE *d = (struct MSVCRT__LDOUBLE *)value; pthreadlocinfo locinfo; const char *beg, *p; int err, ret = 0; @@ -1525,15 +1524,15 @@ int CDECL __STRINGTOLD_L( MSVCRT__LDOUBLE *value, char **endptr, if (endptr) *endptr = (p == beg ? (char*)str : (char*)p); if (p == beg) ret = 4;
- err = fpnum_ldouble(&fp, value); - if (err) ret = (value->x80[2] & 0x7fff ? 2 : 1); + err = fpnum_ldouble(&fp, d); + if (err) ret = (d->exp & 0x7fff ? 2 : 1); return ret; }
/******************************************************************** * __STRINGTOLD (MSVCRT.@) */ -int CDECL __STRINGTOLD( MSVCRT__LDOUBLE *value, char **endptr, const char *str, int flags ) +int CDECL __STRINGTOLD( _LDOUBLE *value, char **endptr, const char *str, int flags ) { return __STRINGTOLD_L( value, endptr, str, flags, NULL ); } @@ -1541,7 +1540,7 @@ int CDECL __STRINGTOLD( MSVCRT__LDOUBLE *value, char **endptr, const char *str, /******************************************************************** * _atoldbl_l (MSVCRT.@) */ -int CDECL _atoldbl_l( MSVCRT__LDOUBLE *value, char *str, _locale_t locale ) +int CDECL _atoldbl_l( _LDOUBLE *value, char *str, _locale_t locale ) { char *endptr; switch(__STRINGTOLD_L( value, &endptr, str, 0, locale )) @@ -1557,7 +1556,7 @@ int CDECL _atoldbl_l( MSVCRT__LDOUBLE *value, char *str, _locale_t locale ) */ int CDECL _atoldbl(_LDOUBLE *value, char *str) { - return _atoldbl_l( (MSVCRT__LDOUBLE*)value, str, NULL ); + return _atoldbl_l( value, str, NULL ); }
/********************************************************************* @@ -2658,29 +2657,30 @@ struct _I10_OUTPUT_DATA { * Native sets last byte of data->str to '0' or '9', I don't know what * it means. Current implementation sets it always to '0'. */ -int CDECL I10_OUTPUT(MSVCRT__LDOUBLE ld80, int prec, int flag, struct _I10_OUTPUT_DATA *data) +int CDECL I10_OUTPUT(_LDOUBLE ld80, int prec, int flag, struct _I10_OUTPUT_DATA *data) { + struct MSVCRT__LDOUBLE *ld = (struct MSVCRT__LDOUBLE *)&ld80; struct fpnum num; double d; char format[8]; char buf[I10_OUTPUT_MAX_PREC+9]; /* 9 = strlen("0.e+0000") + '\0' */ char *p;
- if ((ld80.x80[2] & 0x7fff) == 0x7fff) + if ((ld->exp & 0x7fff) == 0x7fff) { - if (ld80.x80[0] == 0 && ld80.x80[1] == 0x80000000) + if (ld->m == 0x8000000000000000ull) strcpy( data->str, "1#INF" ); else - strcpy( data->str, (ld80.x80[1] & 0x40000000) ? "1#QNAN" : "1#SNAN" ); + strcpy( data->str, (ld->m & 0x4000000000000000ull) ? "1#QNAN" : "1#SNAN" ); data->pos = 1; - data->sign = (ld80.x80[2] & 0x8000) ? '-' : ' '; + data->sign = (ld->exp & 0x8000) ? '-' : ' '; data->len = strlen(data->str); return 0; }
- num.sign = (ld80.x80[2] & 0x8000) ? -1 : 1; - num.exp = (ld80.x80[2] & 0x7fff) - 0x3fff - 63; - num.m = ld80.x80[0] | ((ULONGLONG)ld80.x80[1] << 32); + num.sign = (ld->exp & 0x8000) ? -1 : 1; + num.exp = (ld->exp & 0x7fff) - 0x3fff - 63; + num.m = ld->m; num.mod = FP_ROUND_EVEN; fpnum_double( &num, &d ); TRACE("(%lf %d %x %p)\n", d, prec, flag, data);
On Wed May 7 13:58:11 2025 +0000, Piotr Caban wrote:
I was thinking about something like [0001-msvcrt-Use-_LDOUBLE-type-in-exported-functions.patch](/uploads/5b8330532e8ef0e472babe1e268d9703/0001-msvcrt-Use-_LDOUBLE-type-in-exported-functions.patch) when we talked about fixing it. Could you please update the MR with this patch if it looks good for you?
Thank you! Applied.
This merge request was approved by Piotr Caban.
-
Piotr Caban (＠piotr) -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)