From patchwork Sun Aug 25 18:17:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Peter Ammon X-Patchwork-Id: 96456 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CD2543864836 for ; Sun, 25 Aug 2024 18:18:09 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) by sourceware.org (Postfix) with ESMTPS id D0DFD3858427 for ; Sun, 25 Aug 2024 18:17:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D0DFD3858427 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=ridiculousfish.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ridiculousfish.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D0DFD3858427 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724609866; cv=none; b=aS31KR0QfHJmO8eT8m7GUQplw2ZfKHteLPr79lJZD03X0x2wgYLNnvXdNvy8zmlNpW3xeau97u4Nu3ots26zFXU4rejOMj+yznmha/tqRjNnJUCwzsWig0AwCqNFnTrM8AQe8XJcG3r8pFvrRJsAj2ZENLwTzPZnLfwgGVdOv6k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724609866; c=relaxed/simple; bh=N/8rtJPgeAB4VHj9Fc17JOTS0oHCDU6Q9MBO0F2TlD8=; h=DKIM-Signature:DKIM-Signature:From:Mime-Version:Subject: Message-Id:Date:To; b=Y0aHtnenIQJN0LXzAx9kfq8Y1hX1Lfg1JRYSsp4ZOqc8kwFi+1Mxne/CeVcs8vGZniZQsz7OFYh+8YipoTOdD7cf8qL+7r7Dk5nUmgdZwq8tXqMfZWgDfjP4Ulgc08OIBJ5GV+La+womhWVffg+172U9NlGaAcLqpPTN7ueb9Lo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from phl-compute-06.internal (phl-compute-06.nyi.internal [10.202.2.46]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 86FE2114D725 for ; Sun, 25 Aug 2024 14:17:43 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Sun, 25 Aug 2024 14:17:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= ridiculousfish.com; h=cc:content-transfer-encoding:content-type :content-type:date:date:from:from:in-reply-to:message-id :mime-version:reply-to:subject:subject:to:to; s=fm2; t= 1724609863; x=1724696263; bh=XovmhxSdLmGEHEQa8YM6ndqdsrcV9BDAeHF W3De4gLo=; b=edLZ2i50yl6YGf77feEnV9HCA+455kLkv4hSA1fDxOBsWmngXZY oxAkib2CWOKED1Azh6+IaPSf7kvxRsMkinznAM7wA81JNqM4CiZ6g/w40v9rsX9Q nfUD8LkPelFFXgk1a54TGa2InKIveIENd0PsqewJVrF5RDvUneqccfd5HQRW6pNZ sDX4riIPxDz8HDPCtcMLLqGusaiAiw1tfbobNIWz6C8DvmFAMGdGZ8nN7nNZzTvO yyxCZPRdKz2HRuYXCn1kxLAztR98lgmv4CTeufHcUFMch1fMErncMlFbtMf5MKcp Mp/ub0xxjzoCPRkU41KU3dEbaBFUpbuVP4g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:message-id:mime-version:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1724609863; x=1724696263; bh=XovmhxSdLmGEHEQa8YM6ndqdsrcV 9BDAeHFW3De4gLo=; b=ThnUoJhxpTRNnUsIHT0SWd2JGft8qSM6gKrfj2dnW3pS QZcAVwZyKVS8tSjmMzCNqgMkOH7c+Y/7vqNwxPkZuDg7937VSPp2cTbZBL+9EKLK /SGeUO3opuwbYqgizPqrwNjdqLnyGbyVlcHkI5mO+BMuntT4Lm6YEWiDpcixiiqB pB9tppSDw4V6ctpeRVYsedC0snCqYgRNYCm1kMxUr9Oy7wjHgetg1+/98Ak3bBKf +tGw9lRBrGtr6CE9y/uihSn7JhqU/I42rq03KkD+94m9TjNGxOV1SvScYpzau7oj yo39JfMutwfPp60L/9jWAdxLOmdC79/2vs7IPW0qrg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddviedguddvvdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdp uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecunecujfgurhephfgtgf gguffkfffvofesthhqmhdthhdtjeenucfhrhhomheprfgvthgvrhcutehmmhhonhcuoegt ohhrhiguohhrrghssehrihguihgtuhhlohhushhfihhshhdrtghomheqnecuggftrfgrth htvghrnhepuedujedvffeihfefveefudeftdfgvdffledtudduheeigeelvdegtedvvdeu gfeknecuffhomhgrihhnpehsohhurhgtvgifrghrvgdrohhrghdpghhnuhdrohhrghenuc evlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegtohhrhigu ohhrrghssehrihguihgtuhhlohhushhfihhshhdrtghomhdpnhgspghrtghpthhtohepud dpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhisggtqdgrlhhphhgrsehsohhu rhgtvgifrghrvgdrohhrgh X-ME-Proxy: Feedback-ID: i0ec14495:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Sun, 25 Aug 2024 14:17:42 -0400 (EDT) From: Peter Ammon Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.600.62\)) Subject: [PATCH v1] libio: Fix crash in fputws [BZ #20632] Message-Id: <8CD7FB1C-FCDC-461D-A180-0513852DF24A@ridiculousfish.com> Date: Sun, 25 Aug 2024 11:17:31 -0700 To: libc-alpha@sourceware.org X-Mailer: Apple Mail (2.3774.600.62) X-Spam-Status: No, score=-13.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~patchwork=sourceware.org@sourceware.org This fixes a buffer overflow in wide character string output, reproducing when output fails, such as if the output fd is closed. Wide character output data attempts to maintain the invariant that `_IO_buf_base <= _IO_write_base <= _IO_write_end <= _IO_buf_end` (that is, that the write region is a sub-region of `_IO_buf`). Prior to this commit, this invariant is violated by the `_IO_wfile_overflow` function as so: 1. `_IO_wsetg` is called, assigning `_IO_write_base` to `_IO_buf_base` 2. `_IO_setg` is called, assigning `_IO_buf_base` to a malloc’d buffer. Thus the invariant is violated. The fix is simply to reverse the order: malloc the `_IO_buf` first and then assign `_IO_write_base` to it. We also take this opportunity to defensively guard the initialization of the number of unwritten characters via pointer arithmetic. We now check that the buffer end is not before the buffer beginning; this matches a similar defensive check in the narrow analogue `fileops.c`. Add a test which fails without the fix. Fixes https://sourceware.org/bugzilla/show_bug.cgi?id=20632 Signed-off-by: Peter Ammon --- libio/wfileops.c | 10 +++-- string/Makefile | 1 + string/test-fputs-regression-20632.c | 60 +++++++++++++++++++++++++++ wcsmbs/Makefile | 1 + wcsmbs/test-fputws-regression-20632.c | 20 +++++++++ 5 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 string/test-fputs-regression-20632.c create mode 100644 wcsmbs/test-fputws-regression-20632.c -- 2.46.0 diff --git a/libio/wfileops.c b/libio/wfileops.c index 6de5968358..8a1912cc0e 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -420,14 +420,14 @@ _IO_wfile_overflow (FILE *f, wint_t wch) { _IO_wdoallocbuf (f); _IO_free_wbackup_area (f); - _IO_wsetg (f, f->_wide_data->_IO_buf_base, - f->_wide_data->_IO_buf_base, f->_wide_data->_IO_buf_base); if (f->_IO_write_base == NULL) { _IO_doallocbuf (f); _IO_setg (f, f->_IO_buf_base, f->_IO_buf_base, f->_IO_buf_base); } + _IO_wsetg (f, f->_wide_data->_IO_buf_base, + f->_wide_data->_IO_buf_base, f->_wide_data->_IO_buf_base); } else { @@ -958,7 +958,7 @@ _IO_wfile_xsputn (FILE *f, const void *data, size_t n) const wchar_t *s = (const wchar_t *) data; size_t to_do = n; int must_flush = 0; - size_t count; + size_t count = 0; if (n <= 0) return 0; @@ -967,7 +967,6 @@ _IO_wfile_xsputn (FILE *f, const void *data, size_t n) (or the filebuf is unbuffered), use sys_write directly. */ /* First figure out how much space is available in the buffer. */ - count = f->_wide_data->_IO_write_end - f->_wide_data->_IO_write_ptr; if ((f->_flags & _IO_LINE_BUF) && (f->_flags & _IO_CURRENTLY_PUTTING)) { count = f->_wide_data->_IO_buf_end - f->_wide_data->_IO_write_ptr; @@ -985,6 +984,9 @@ _IO_wfile_xsputn (FILE *f, const void *data, size_t n) } } } + else if (f->_wide_data->_IO_write_end > f->_wide_data->_IO_write_ptr) + count = f->_wide_data->_IO_write_end - f->_wide_data->_IO_write_ptr; /* Space available. */ + /* Then fill the buffer. */ if (count > 0) { diff --git a/string/Makefile b/string/Makefile index 1dff405c27..f15ed880d3 100644 --- a/string/Makefile +++ b/string/Makefile @@ -164,6 +164,7 @@ tests := \ test-mempcpy \ test-memrchr \ test-memset \ + test-fputs-regression-20632 \ test-rawmemchr \ test-sig_np \ test-stpcpy \ diff --git a/string/test-fputs-regression-20632.c b/string/test-fputs-regression-20632.c new file mode 100644 index 0000000000..2cf779eeec --- /dev/null +++ b/string/test-fputs-regression-20632.c @@ -0,0 +1,60 @@ +/* Regression test for 20632. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#define TEST_MAIN +#ifndef WIDE +# define TEST_NAME "fputs-regression-20632" +#else +# define TEST_NAME "fputws-regression-20632" +#endif /* WIDE */ +#include "test-string.h" + +#ifndef WIDE +# define CHAR char +# define FPUTS fputs +# define TEXT "0123456789ABCDEF" +#else +# include +# define CHAR wchar_t +# define FPUTS fputws +# define TEXT L"0123456789ABCDEF" +#endif + + +int main(void) { + /* Close stderr */ + close(2); + + /* Output long string */ + const int sz = 4096; + CHAR *buff = calloc(sz+1, sizeof *buff); + for (int i=0; i < sz; i++) buff[i] = (CHAR)'x'; + buff[sz] = (CHAR)'\0'; + FPUTS(buff, stderr); + + /* Output shorter string */ + for (int i=0; i < 1024; i++) { + FPUTS(TEXT, stderr); + + /* Call malloc, which should not crash. + However it will if malloc's function pointers + have been stomped. */ + free(malloc(1)); + } + return 0; +} diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile index 63adf0e8ef..ccc7aef866 100644 --- a/wcsmbs/Makefile +++ b/wcsmbs/Makefile @@ -146,6 +146,7 @@ tests := \ test-c8rtomb \ test-char-types \ test-mbrtoc8 \ + test-fputws-regression-20632 \ test-wcpcpy \ test-wcpncpy \ test-wcscat \ diff --git a/wcsmbs/test-fputws-regression-20632.c b/wcsmbs/test-fputws-regression-20632.c new file mode 100644 index 0000000000..ab18c3e497 --- /dev/null +++ b/wcsmbs/test-fputws-regression-20632.c @@ -0,0 +1,20 @@ +/* Regression test for 20632. Wide character output doesn't crash on a closed fd. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#define WIDE 1 +#include "../string/test-fputs-regression-20632.c"