From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14747 invoked by alias); 2 Sep 2016 13:19:57 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 14725 invoked by uid 89); 2 Sep 2016 13:19:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=cnt, race, realloc, 7807 X-HELO: mx1.redhat.com Date: Fri, 02 Sep 2016 13:19:00 -0000 To: libc-alpha@sourceware.org Subject: [PATCH] vfscanf: Avoid multiple reads of multi-byte character width User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-Id: <20160902131953.69ABB40191DBC@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) X-SW-Source: 2016-09/txt/msg00026.txt.bz2 This avoids a race condition if the process-global locale is changed while vfscanf is running. MB_LEN_MAX is always larger than MB_CUR_MAX, so we might realloc earlier than necessary (but even MB_CUR_MAX could be larger than the minimum required space). The existing length was a bit questionable because str + MB_LEN_MAX might point past the end of the buffer. 2016-09-02 Florian Weimer * stdio-common/vfscanf.c (_IO_vfwscanf): Use MB_LEN_MAX instead of MB_CUR_MAX to avoid race condition. Avoid pointer arithmetic outside of allocated array. diff --git a/stdio-common/vfscanf.c b/stdio-common/vfscanf.c index 8cd5955..2b7093e 100644 --- a/stdio-common/vfscanf.c +++ b/stdio-common/vfscanf.c @@ -757,7 +757,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, size_t n; if (!(flags & SUPPRESS) && (flags & POSIX_MALLOC) - && str + MB_CUR_MAX >= *strptr + strsize) + && MB_LEN_MAX >= *strptr + strsize - str) { /* We have to enlarge the buffer if the `m' flag was given. */ @@ -769,7 +769,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, { /* Can't allocate that much. Last-ditch effort. */ newstr = (char *) realloc (*strptr, - strleng + MB_CUR_MAX); + strleng + MB_LEN_MAX); if (newstr == NULL) { /* c can't have `a' flag, only `m'. */ @@ -780,7 +780,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, { *strptr = newstr; str = newstr + strleng; - strsize = strleng + MB_CUR_MAX; + strsize = strleng + MB_LEN_MAX; } } else @@ -1048,7 +1048,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, size_t n; if (!(flags & SUPPRESS) && (flags & MALLOC) - && str + MB_CUR_MAX >= *strptr + strsize) + && MB_LEN_MAX >= *strptr + strsize - str) { /* We have to enlarge the buffer if the `a' or `m' flag was given. */ @@ -1061,7 +1061,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, /* Can't allocate that much. Last-ditch effort. */ newstr = (char *) realloc (*strptr, - strleng + MB_CUR_MAX); + strleng + MB_LEN_MAX); if (newstr == NULL) { if (flags & POSIX_MALLOC) @@ -1081,7 +1081,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, { *strptr = newstr; str = newstr + strleng; - strsize = strleng + MB_CUR_MAX; + strsize = strleng + MB_LEN_MAX; } } else @@ -1097,7 +1097,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, if (__glibc_unlikely (n == (size_t) -1)) encode_error (); - assert (n <= MB_CUR_MAX); + assert (n <= MB_LEN_MAX); str += n; } #else @@ -2675,7 +2675,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, /* Possibly correct character, just not enough input. */ ++cnt; - assert (cnt < MB_CUR_MAX); + assert (cnt < MB_LEN_MAX); continue; } cnt = 0; @@ -2827,7 +2827,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, if (!(flags & SUPPRESS)) { if ((flags & MALLOC) - && str + MB_CUR_MAX >= *strptr + strsize) + && MB_LEN_MAX >= *strptr + strsize - str) { /* Enlarge the buffer. */ size_t strleng = str - *strptr; @@ -2839,7 +2839,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, /* Can't allocate that much. Last-ditch effort. */ newstr = (char *) realloc (*strptr, - strleng + MB_CUR_MAX); + strleng + MB_LEN_MAX); if (newstr == NULL) { if (flags & POSIX_MALLOC) @@ -2859,7 +2859,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, { *strptr = newstr; str = newstr + strleng; - strsize = strleng + MB_CUR_MAX; + strsize = strleng + MB_LEN_MAX; } } else @@ -2875,7 +2875,7 @@ _IO_vfscanf_internal (_IO_FILE *s, const char *format, _IO_va_list argptr, if (__glibc_unlikely (n == (size_t) -1)) encode_error (); - assert (n <= MB_CUR_MAX); + assert (n <= MB_LEN_MAX); str += n; } while (--width > 0 && inchar () != WEOF);