From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125386 invoked by alias); 1 Jun 2018 09:23:47 -0000 Mailing-List: contact libc-stable-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: List-Archive: Sender: libc-stable-owner@sourceware.org Received: (qmail 125253 invoked by uid 89); 1 Jun 2018 09:23:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.99.4 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=cherry X-Spam-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=ham version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Jun 2018 09:23:36 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A66A8DF23D for ; Fri, 1 Jun 2018 09:23:35 +0000 (UTC) Received: from oldenburg.str.redhat.com (dhcp-192-212.str.redhat.com [10.33.192.212]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4D27060C8C for ; Fri, 1 Jun 2018 09:23:35 +0000 (UTC) Received: by oldenburg.str.redhat.com (Postfix, from userid 1000) id 66C50401241AC; Fri, 1 Jun 2018 11:23:34 +0200 (CEST) Date: Mon, 01 Jan 2018 00:00:00 -0000 To: libc-stable@sourceware.org Subject: [2.27 COMMITTED] libio: Avoid _allocate_buffer, _free_buffer function pointers [BZ #23236] 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: <20180601092334.66C50401241AC@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 01 Jun 2018 09:23:35 +0000 (UTC) X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00000.txt.bz2 These unmangled function pointers reside on the heap and could be targeted by exploit writers, effectively bypassing libio vtable validation. Instead, we ignore these pointers and always call malloc or free. In theory, this is a backwards-incompatible change, but using the global heap instead of the user-supplied callback functions should have little application impact. (The old libstdc++ implementation exposed this functionality via a public, undocumented constructor in its strstreambuf class.) (cherry picked from commit 4e8a6346cd3da2d88bbad745a1769260d36f2783) 2018-06-01 Florian Weimer [BZ #23236] * libio/strfile.h (struct _IO_str_fields): Rename members to discourage their use and add comment. (_IO_STR_DYNAMIC): Remove unused macro. * libio/strops.c (_IO_str_init_static_internal): Do not use callback pointers. Call malloc and free. (_IO_str_overflow): Do not use callback pointers. Call malloc and free. (enlarge_userbuf): Likewise. (_IO_str_finish): Call free. * libio/wstrops.c (_IO_wstr_init_static): Initialize _allocate_buffer_unused. (_IO_wstr_overflow): Do not use callback pointers. Call malloc and free. (enlarge_userbuf): Likewise. (_IO_wstr_finish): Call free. * debug/vasprintf_chk.c (__vasprintf_chk): Initialize _allocate_buffer_unused, _free_buffer_unused. * libio/memstream.c (__open_memstream): Likewise. * libio/vasprintf.c (_IO_vasprintf): Likewise. * libio/wmemstream.c (open_wmemstream): Likewise. diff --git a/NEWS b/NEWS index 2c58d073a3..4c1dbda104 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,7 @@ The following bugs are resolved with this release: [23152] gd_GB: Fix typo in "May" (abbreviated) [23166] sunrpc: Remove stray exports without --enable-obsolete-rpc [23196] __mempcpy_avx512_no_vzeroupper mishandles large copies + [23236] Harden function pointers in _IO_str_fields Version 2.27 diff --git a/debug/vasprintf_chk.c b/debug/vasprintf_chk.c index a00ef771e6..3eb64617fd 100644 --- a/debug/vasprintf_chk.c +++ b/debug/vasprintf_chk.c @@ -55,8 +55,8 @@ __vasprintf_chk (char **result_ptr, int flags, const char *format, _IO_JUMPS (&sf._sbf) = &_IO_str_jumps; _IO_str_init_static_internal (&sf, string, init_string_size, string); sf._sbf._f._flags &= ~_IO_USER_BUF; - sf._s._allocate_buffer = (_IO_alloc_type) malloc; - sf._s._free_buffer = (_IO_free_type) free; + sf._s._allocate_buffer_unused = (_IO_alloc_type) malloc; + sf._s._free_buffer_unused = (_IO_free_type) free; /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n can only come from read-only format strings. */ diff --git a/libio/memstream.c b/libio/memstream.c index d86befcc02..c5c7c2f6db 100644 --- a/libio/memstream.c +++ b/libio/memstream.c @@ -90,8 +90,8 @@ __open_memstream (char **bufloc, _IO_size_t *sizeloc) _IO_JUMPS_FILE_plus (&new_f->fp._sf._sbf) = &_IO_mem_jumps; _IO_str_init_static_internal (&new_f->fp._sf, buf, _IO_BUFSIZ, buf); new_f->fp._sf._sbf._f._flags &= ~_IO_USER_BUF; - new_f->fp._sf._s._allocate_buffer = (_IO_alloc_type) malloc; - new_f->fp._sf._s._free_buffer = (_IO_free_type) free; + new_f->fp._sf._s._allocate_buffer_unused = (_IO_alloc_type) malloc; + new_f->fp._sf._s._free_buffer_unused = (_IO_free_type) free; new_f->fp.bufloc = bufloc; new_f->fp.sizeloc = sizeloc; diff --git a/libio/strfile.h b/libio/strfile.h index 68dfcbfe83..52a085e580 100644 --- a/libio/strfile.h +++ b/libio/strfile.h @@ -31,8 +31,11 @@ typedef void (*_IO_free_type) (void*); struct _IO_str_fields { - _IO_alloc_type _allocate_buffer; - _IO_free_type _free_buffer; + /* These members are preserved for ABI compatibility. The glibc + implementation always calls malloc/free for user buffers if + _IO_USER_BUF or _IO_FLAGS2_USER_WBUF are not set. */ + _IO_alloc_type _allocate_buffer_unused; + _IO_free_type _free_buffer_unused; }; /* This is needed for the Irix6 N32 ABI, which has a 64 bit off_t type, @@ -52,10 +55,6 @@ typedef struct _IO_strfile_ struct _IO_str_fields _s; } _IO_strfile; -/* dynamic: set when the array object is allocated (or reallocated) as - necessary to hold a character sequence that can change in length. */ -#define _IO_STR_DYNAMIC(FP) ((FP)->_s._allocate_buffer != (_IO_alloc_type)0) - /* frozen: set when the program has requested that the array object not be altered, reallocated, or freed. */ #define _IO_STR_FROZEN(FP) ((FP)->_f._IO_file_flags & _IO_USER_BUF) diff --git a/libio/strops.c b/libio/strops.c index ac995c830e..5fb38976e3 100644 --- a/libio/strops.c +++ b/libio/strops.c @@ -61,7 +61,7 @@ _IO_str_init_static_internal (_IO_strfile *sf, char *ptr, _IO_size_t size, fp->_IO_read_end = end; } /* A null _allocate_buffer function flags the strfile as being static. */ - sf->_s._allocate_buffer = (_IO_alloc_type) 0; + sf->_s._allocate_buffer_unused = (_IO_alloc_type) 0; } void @@ -103,8 +103,7 @@ _IO_str_overflow (_IO_FILE *fp, int c) _IO_size_t new_size = 2 * old_blen + 100; if (new_size < old_blen) return EOF; - new_buf - = (char *) (*((_IO_strfile *) fp)->_s._allocate_buffer) (new_size); + new_buf = malloc (new_size); if (new_buf == NULL) { /* __ferror(fp) = 1; */ @@ -113,7 +112,7 @@ _IO_str_overflow (_IO_FILE *fp, int c) if (old_buf) { memcpy (new_buf, old_buf, old_blen); - (*((_IO_strfile *) fp)->_s._free_buffer) (old_buf); + free (old_buf); /* Make sure _IO_setb won't try to delete _IO_buf_base. */ fp->_IO_buf_base = NULL; } @@ -182,15 +181,14 @@ enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading) _IO_size_t newsize = offset + 100; char *oldbuf = fp->_IO_buf_base; - char *newbuf - = (char *) (*((_IO_strfile *) fp)->_s._allocate_buffer) (newsize); + char *newbuf = malloc (newsize); if (newbuf == NULL) return 1; if (oldbuf != NULL) { memcpy (newbuf, oldbuf, _IO_blen (fp)); - (*((_IO_strfile *) fp)->_s._free_buffer) (oldbuf); + free (oldbuf); /* Make sure _IO_setb won't try to delete _IO_buf_base. */ fp->_IO_buf_base = NULL; @@ -346,7 +344,7 @@ void _IO_str_finish (_IO_FILE *fp, int dummy) { if (fp->_IO_buf_base && !(fp->_flags & _IO_USER_BUF)) - (((_IO_strfile *) fp)->_s._free_buffer) (fp->_IO_buf_base); + free (fp->_IO_buf_base); fp->_IO_buf_base = NULL; _IO_default_finish (fp, 0); diff --git a/libio/vasprintf.c b/libio/vasprintf.c index 390a63d124..0bb217e46e 100644 --- a/libio/vasprintf.c +++ b/libio/vasprintf.c @@ -54,8 +54,8 @@ _IO_vasprintf (char **result_ptr, const char *format, _IO_va_list args) _IO_JUMPS (&sf._sbf) = &_IO_str_jumps; _IO_str_init_static_internal (&sf, string, init_string_size, string); sf._sbf._f._flags &= ~_IO_USER_BUF; - sf._s._allocate_buffer = (_IO_alloc_type) malloc; - sf._s._free_buffer = (_IO_free_type) free; + sf._s._allocate_buffer_unused = (_IO_alloc_type) malloc; + sf._s._free_buffer_unused = (_IO_free_type) free; ret = _IO_vfprintf (&sf._sbf._f, format, args); if (ret < 0) { diff --git a/libio/wmemstream.c b/libio/wmemstream.c index c962071d26..f4c6e75246 100644 --- a/libio/wmemstream.c +++ b/libio/wmemstream.c @@ -92,8 +92,8 @@ open_wmemstream (wchar_t **bufloc, _IO_size_t *sizeloc) _IO_wstr_init_static (&new_f->fp._sf._sbf._f, buf, _IO_BUFSIZ / sizeof (wchar_t), buf); new_f->fp._sf._sbf._f._flags2 &= ~_IO_FLAGS2_USER_WBUF; - new_f->fp._sf._s._allocate_buffer = (_IO_alloc_type) malloc; - new_f->fp._sf._s._free_buffer = (_IO_free_type) free; + new_f->fp._sf._s._allocate_buffer_unused = (_IO_alloc_type) malloc; + new_f->fp._sf._s._free_buffer_unused = (_IO_free_type) free; new_f->fp.bufloc = bufloc; new_f->fp.sizeloc = sizeloc; diff --git a/libio/wstrops.c b/libio/wstrops.c index a3374a7b15..0839a70bfb 100644 --- a/libio/wstrops.c +++ b/libio/wstrops.c @@ -63,7 +63,7 @@ _IO_wstr_init_static (_IO_FILE *fp, wchar_t *ptr, _IO_size_t size, fp->_wide_data->_IO_read_end = end; } /* A null _allocate_buffer function flags the strfile as being static. */ - (((_IO_strfile *) fp)->_s._allocate_buffer) = (_IO_alloc_type)0; + (((_IO_strfile *) fp)->_s._allocate_buffer_unused) = (_IO_alloc_type)0; } _IO_wint_t @@ -95,9 +95,7 @@ _IO_wstr_overflow (_IO_FILE *fp, _IO_wint_t c) || __glibc_unlikely (new_size > SIZE_MAX / sizeof (wchar_t))) return EOF; - new_buf - = (wchar_t *) (*((_IO_strfile *) fp)->_s._allocate_buffer) (new_size - * sizeof (wchar_t)); + new_buf = malloc (new_size * sizeof (wchar_t)); if (new_buf == NULL) { /* __ferror(fp) = 1; */ @@ -106,7 +104,7 @@ _IO_wstr_overflow (_IO_FILE *fp, _IO_wint_t c) if (old_buf) { __wmemcpy (new_buf, old_buf, old_wblen); - (*((_IO_strfile *) fp)->_s._free_buffer) (old_buf); + free (old_buf); /* Make sure _IO_setb won't try to delete _IO_buf_base. */ fp->_wide_data->_IO_buf_base = NULL; } @@ -186,16 +184,14 @@ enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading) return 1; wchar_t *oldbuf = wd->_IO_buf_base; - wchar_t *newbuf - = (wchar_t *) (*((_IO_strfile *) fp)->_s._allocate_buffer) (newsize - * sizeof (wchar_t)); + wchar_t *newbuf = malloc (newsize * sizeof (wchar_t)); if (newbuf == NULL) return 1; if (oldbuf != NULL) { __wmemcpy (newbuf, oldbuf, _IO_wblen (fp)); - (*((_IO_strfile *) fp)->_s._free_buffer) (oldbuf); + free (oldbuf); /* Make sure _IO_setb won't try to delete _IO_buf_base. */ wd->_IO_buf_base = NULL; @@ -357,7 +353,7 @@ void _IO_wstr_finish (_IO_FILE *fp, int dummy) { if (fp->_wide_data->_IO_buf_base && !(fp->_flags2 & _IO_FLAGS2_USER_WBUF)) - (((_IO_strfile *) fp)->_s._free_buffer) (fp->_wide_data->_IO_buf_base); + free (fp->_wide_data->_IO_buf_base); fp->_wide_data->_IO_buf_base = NULL; _IO_wdefault_finish (fp, 0);