From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40957 invoked by alias); 7 Jun 2017 10:41:22 -0000 Mailing-List: contact newlib-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: newlib-owner@sourceware.org Received: (qmail 40374 invoked by uid 89); 7 Jun 2017 10:41:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD,UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=Hx-languages-length:3946 X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Jun 2017 10:41:19 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4FC162B for ; Wed, 7 Jun 2017 03:41:22 -0700 (PDT) Received: from [10.2.206.52] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F06F73F587 for ; Wed, 7 Jun 2017 03:41:21 -0700 (PDT) Subject: Re: Fix modification of string literal by swprintf To: newlib@sourceware.org References: <608ad190-c35e-76b1-99d3-e96dd64d85e2@foss.arm.com> <20170607100157.GB18287@calimero.vinschen.de> From: Thomas Preudhomme Message-ID: <50149eef-d512-4bb0-3d5e-fcd6302b8083@foss.arm.com> Date: Wed, 07 Jun 2017 10:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170607100157.GB18287@calimero.vinschen.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017/txt/msg00390.txt.bz2 Hi Corinna, On 07/06/17 11:01, Corinna Vinschen wrote: > On Jun 5 09:59, Thomas Preudhomme wrote: >> Don't over-read memory returned by _DTOA_R, and never write to it >> since the result might be a string literal. >> >> For example, when doing: >> swprintf(tt, 20, L"%.*f", 6, 0.0); >> >> we will get back "0". >> >> Instead, write the result returned by _DTOA_R to the output buffer. >> After this, write the 0 chars directly to the the output buffer >> (if there are any). This also has the (marginal) advantage that >> we read/write less memory overall. >> >> The patch, contributed by Silviu Baranga was tested against libcxx testsuite >> and showed no regression. Please find the patch in git format-patch format >> in attachment. >> >> Best regards, >> >> Thomas > >> >From 7a31cfb01a0b089daf2bed93b742b6edbf4cba0c Mon Sep 17 00:00:00 2001 >> From: Silviu Baranga >> Date: Mon, 5 Jun 2017 09:54:42 +0100 >> Subject: [PATCH] Don't overread or write memory returned by _DTOA_R >> >> Don't over-read memory returned by _DTOA_R, and never write to it >> since the result might be a string literal. >> >> For example, when doing: >> swprintf(tt, 20, L"%.*f", 6, 0.0); >> >> we will get back "0". >> >> Instead, write the result returned by _DTOA_R to the output buffer. >> After this, write the 0 chars directly to the the output buffer >> (if there are any). This also has the (marginal) advantage that >> we read/write less memory overall. >> --- >> newlib/libc/locale/setlocale.h | 2 +- >> newlib/libc/stdio/vfwprintf.c | 24 ++++++++++++------------ >> 2 files changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/newlib/libc/locale/setlocale.h b/newlib/libc/locale/setlocale.h >> index 85a38d5..1440d0e 100644 >> --- a/newlib/libc/locale/setlocale.h >> +++ b/newlib/libc/locale/setlocale.h >> @@ -227,7 +227,7 @@ __get_locale_r (struct _reent *r) >> _ELIDABLE_INLINE struct __locale_t * >> __get_current_locale (void) >> { >> - return _REENT->_locale ?: __get_global_locale (); >> + return _REENT->_locale ?: __get_global_locale (); // version for !_MB_CAPABLE? >> } > > This doesn't belong here. Also, the code is already fine for !_MB_CAPABLE > as well. My bad, this was a note I made to myself when reading on MB_CAPABLE some time ago. This does not belong to the patch and is indeed wrong. > >> /* Only access fixed "C" locale using this function. Fake for !_MB_CAPABLE >> diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c >> index f0179a0..1bec9b2 100644 >> --- a/newlib/libc/stdio/vfwprintf.c >> +++ b/newlib/libc/stdio/vfwprintf.c >> @@ -1627,13 +1627,20 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, >> >> { >> char *digits, *bp, *rve; >> -#ifndef _MB_CAPABLE >> int i; >> -#endif >> >> digits = _DTOA_R (data, value, mode, ndigits, decpt, &dsgn, &rve); >> >> +#ifdef _MB_CAPABLE >> + _mbsnrtowcs_r (data, buf, (const char **) &digits, rve - digits, >> + len, NULL); >> +#else >> + for (i = 0; i < rve - digits && i < len; ++i) >> + buf[i] = (wchar_t) digits[i]; >> +#endif >> + >> if ((ch != L'g' && ch != L'G') || flags & ALT) { /* Print trailing zeros */ >> + char *padding = rve; >> bp = digits + ndigits; >> if (ch == L'f' || ch == L'F') { >> if (*digits == L'0' && value) >> @@ -1642,18 +1649,11 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, >> } >> if (value == 0) /* kludge for __dtoa irregularity */ >> rve = bp; >> - while (rve < bp) >> - *rve++ = '0'; >> - } >> >> + for (i = padding - digits; i < rve - digits && i < len; ++i) >> + buf[i] = L'0'; > > Appending zeros here without incrementing rve... > >> + } >> *length = rve - digits; /* full length of the string */ > > ...leads to incorrect setting of *length here. Or am I missing > something? I'll transmit to Silviu for him to answer. Thanks for the comments. Best regards, Thomas