[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#7357: csplit: memory exhausted when using stdout / pipe instead of a
From: |
Jim Meyering |
Subject: |
bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file |
Date: |
Wed, 10 Nov 2010 21:56:28 +0100 |
Paul Eggert wrote:
>> + unsigned int max_digit_string_len
>> + = (suffix
>> + ? max_out (suffix)
>> + : MAX (INT_STRLEN_BOUND (unsigned int), digits));
>
> That should be size_t, not unsigned int, since max_out
> returns a size_t, and it could return a value greater than
Good catch.
In addition, it should be using snprintf, not sprintf, on principle.
> UINT_MAX. For example, the user might run "csplit -b %4294967296d"
> and on a 64-bit host max_out will return UINTMAX + 1.
>
> While we're on the subject of undefined printf behavior, perhaps
> we should be rejecting any attempt to use a printf format like
> %4294967296d that uses a width or precision greater than INT_MAX?
> POSIX seems to say that such a format should work, but I'll bet
> nobody does it right (glibc doesn't). For safety we might want
> to be truncating large widths or precisions to INT_MAX, or
> rejecting them.
Here are two patches. First follows your suggestions.
The second cleans up my just-added test.
>From e51561440446cd23c8c60b96d4712263423fd39c Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 10 Nov 2010 21:30:26 +0100
Subject: [PATCH 1/2] csplit: don't misbehave given a format like %4294967296d
* src/csplit.c (main): Use size_t for length, not unsigned int.
Also, reject options that would require a file name buffer longer
than INT_MAX.
* tests/misc/csplit-abuse: Test for this.
* tests/Makefile.am (TESTS): Add misc/csplit-abuse.
Suggested by Paul Eggert.
---
src/csplit.c | 8 ++++++--
tests/Makefile.am | 1 +
tests/misc/csplit-abuse | 28 ++++++++++++++++++++++++++++
3 files changed, 35 insertions(+), 2 deletions(-)
create mode 100755 tests/misc/csplit-abuse
diff --git a/src/csplit.c b/src/csplit.c
index 57543f0..e510cb5 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -1372,11 +1372,15 @@ main (int argc, char **argv)
usage (EXIT_FAILURE);
}
- unsigned int max_digit_string_len
+ size_t max_digit_string_len
= (suffix
? max_out (suffix)
: MAX (INT_STRLEN_BOUND (unsigned int), digits));
- filename_space = xmalloc (strlen (prefix) + max_digit_string_len + 1);
+
+ size_t buf_len = strlen (prefix) + max_digit_string_len + 1;
+ if (INT_MAX < buf_len || INT_MAX < max_digit_string_len)
+ error (EXIT_FAILURE, 0, _("invalid options"));
+ filename_space = xmalloc (buf_len);
set_input_file (argv[optind++]);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a3a30b6..ceb8de1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -173,6 +173,7 @@ TESTS = \
misc/comm \
misc/csplit \
misc/csplit-1000 \
+ misc/csplit-abuse \
misc/date-sec \
misc/dircolors \
misc/df \
diff --git a/tests/misc/csplit-abuse b/tests/misc/csplit-abuse
new file mode 100755
index 0000000..3448b9a
--- /dev/null
+++ b/tests/misc/csplit-abuse
@@ -0,0 +1,28 @@
+#!/bin/sh
+# ensure that an outrageous format evokes an error
+
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program 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 General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+test "$VERBOSE" = yes && csplit --version
+
+echo csplit: invalid options > exp || framework_failure_
+
+# Require failure.
+csplit - -b %4294967295d x 1 2> err && fail=1
+compare exp err || fail=1
+
+Exit $fail
--
1.7.3.2.4.g60aa9
>From d2c441bb125a18ad949f344db2c192469e2972bb Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 10 Nov 2010 21:54:57 +0100
Subject: [PATCH 2/2] tests: fix comments and --version invocation in new test
* tests/misc/csplit-1000: Fix comments and --version invocation.
---
tests/misc/csplit-1000 | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/misc/csplit-1000 b/tests/misc/csplit-1000
index accbe46..259d514 100755
--- a/tests/misc/csplit-1000
+++ b/tests/misc/csplit-1000
@@ -1,5 +1,5 @@
#!/bin/sh
-# various csplit tests
+# cause a 1-byte heap buffer overrun
# Copyright (C) 2010 Free Software Foundation, Inc.
@@ -17,7 +17,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
. "${srcdir=.}/init.sh"; path_prepend_ ../src
-test "$VERBOSE" = yes && FIXME --version
+test "$VERBOSE" = yes && csplit --version
# Before coreutils-8.7, this would overrun the 6-byte filename_space buffer.
# It's hard to detect that without using valgrind, so here, we simply
--
1.7.3.2.4.g60aa9
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, (continued)
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, Paul Eggert, 2010/11/10
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, Eric Blake, 2010/11/10
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, Paul Eggert, 2010/11/10
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, Jim Meyering, 2010/11/11
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, Voelker, Bernhard, 2010/11/11
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, Jim Meyering, 2010/11/11
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file,
Jim Meyering <=
- bug#7357: csplit: memory exhausted when using stdout / pipe instead of a file, Jim Meyering, 2010/11/10