Cristian Cadar <[EMAIL PROTECTED]> wrote:
> We found a crash bug in paste, due to an unbounded buffer overflow.
> The bug is similar to the ptx bug that we reported earlier, and is due
> to a lone backslash following the -d flag.
> Here is an input that crashes libc on my machine:
>
> $ paste -d\\ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> *** glibc detected *** paste: free(): invalid next size (normal):
> 0x09035888 ***
>
> The problem seems to be in collapse_escapes() which when given a lone
> backslash, incorrectly advances 'strptr' past the end of the string, and
> continues copying from there, overflowing the 'delims' buffer.
>
> As usual, we appreciate your confirmation of the bug.
Hi guys,
Thank you, again!
That bug has been there from the beginning (November 1992, when I first
imported the GNU textutils into a CVS repository).
Here's the patch I expect to push:
paste -d\\: avoid heap overrun for backslash at end of delim list
* src/paste.c: Include "quotearg.h".
(collapse_escapes): Handle backslash-escaped backslash explicitly.
Handle unescaped backslash at end of string by returning nonzero,
rather than by overrunning memory.
(main): Diagnose an invalid delimiter list -- carefully.
Reported by Cristian Cadar, Daniel Dunbar and Dawson Engler.
* tests/misc/paste-no-nl (delim-bs): Add a test to demonstrate the
heap smashing capability.
(delim-bs2): Prior to coreutils-5.1.2, this bug was a little harder
to demonstrate: it would corrupt a first-argument containing e.g., \b
* NEWS: Mention the bug fix.
Signed-off-by: Jim Meyering <[EMAIL PROTECTED]>
---
NEWS | 3 +++
src/paste.c | 34 ++++++++++++++++++++++++++++++----
tests/misc/paste-no-nl | 27 ++++++++++++++++++++++-----
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/NEWS b/NEWS
index 5250ed8..4ac4413 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ GNU coreutils NEWS -*-
outline -*-
when the destination had two or more hard links. It no longer does that.
[bug introduced in coreutils-5.3.0]
+ "paste -d'\' file" no longer overruns memory (heap since coreutils-5.1.2,
+ stack before then) [bug present in the original version, in 1992]
+
"ptx -F'\' long-file-name" would overrun a malloc'd buffer and corrupt
the heap. That was triggered by a lone backslash (or odd number of them)
at the end of the option argument to --flag-truncation=STRING (-F),
diff --git a/src/paste.c b/src/paste.c
index 3cb84cf..20d6953 100644
--- a/src/paste.c
+++ b/src/paste.c
@@ -1,5 +1,5 @@
/* paste - merge lines of files
- Copyright (C) 1997-2005 Free Software Foundation, Inc.
+ Copyright (C) 1997-2005, 2008 Free Software Foundation, Inc.
Copyright (C) 1984 David M. Ihnat
This program is free software: you can redistribute it and/or modify
@@ -42,6 +42,7 @@
#include <sys/types.h>
#include "system.h"
#include "error.h"
+#include "quotearg.h"
/* The official name of this program (e.g., no `g' prefix). */
#define PROGRAM_NAME "paste"
@@ -79,12 +80,17 @@ static struct option const longopts[] =
/* Set globals delims and delim_end. Copy STRPTR to DELIMS, converting
backslash representations of special characters in STRPTR to their actual
values. The set of possible backslash characters has been expanded beyond
- that recognized by the Unix version. */
+ that recognized by the Unix version.
+ Return 0 upon success.
+ If the string ends in an odd number of backslashes, ignore the
+ final backslash and return nonzero. */
-static void
+static int
collapse_escapes (char const *strptr)
{
char *strout = xstrdup (strptr);
+ bool backslash_at_end = false;
+
delims = strout;
while (*strptr)
@@ -123,6 +129,14 @@ collapse_escapes (char const *strptr)
*strout++ = '\v';
break;
+ case '\\':
+ *strout++ = '\\';
+ break;
+
+ case '\0':
+ backslash_at_end = true;
+ goto done;
+
default:
*strout++ = *strptr;
break;
@@ -130,7 +144,11 @@ collapse_escapes (char const *strptr)
strptr++;
}
}
+
+ done:;
+
delim_end = strout;
+ return backslash_at_end ? 1 : 0;
}
/* Report a write error and exit. */
@@ -481,7 +499,15 @@ main (int argc, char **argv)
if (optind == argc)
argv[argc++] = "-";
- collapse_escapes (delim_arg);
+ if (collapse_escapes (delim_arg))
+ {
+ /* Don't use the default quoting style, because that would double the
+ number of displayed backslashes, making the diagnostic look bogus. */
+ set_quoting_style (NULL, escape_quoting_style);
+ error (EXIT_FAILURE, 0,
+ _("delimiter list ends with an unescaped backslash: %s"),
+ quotearg_colon (delim_arg));
+ }
if (!serial_merge)
ok = paste_parallel (argc - optind, &argv[optind]);
diff --git a/tests/misc/paste-no-nl b/tests/misc/paste-no-nl
index ec23991..244bc2e 100755
--- a/tests/misc/paste-no-nl
+++ b/tests/misc/paste-no-nl
@@ -1,8 +1,8 @@
#!/bin/sh
# -*- perl -*-
-# Ensure that paste properly handles files lacking a final newline.
+# Test paste.
-# Copyright (C) 2003, 2005, 2007 Free Software Foundation, Inc.
+# Copyright (C) 2003, 2005, 2007-2008 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
@@ -28,13 +28,15 @@ use strict;
(my $program_name = $0) =~ s|.*/||;
-$ENV{PROG} = 'paste';
-
# Turn off localization of executable's ouput.
@ENV{qw(LANGUAGE LANG LC_ALL)} = ('C') x 3;
+my $prog = 'paste';
+my $msg = "$prog: delimiter string ends with an odd number of backslashes: ";
+
my @Tests =
(
+ # Ensure that paste properly handles files lacking a final newline.
['no-nl-1', {IN=>"a"}, {IN=>"b"}, {OUT=>"a\tb\n"}],
['no-nl-2', {IN=>"a\n"}, {IN=>"b"}, {OUT=>"a\tb\n"}],
['no-nl-3', {IN=>"a"}, {IN=>"b\n"}, {OUT=>"a\tb\n"}],
@@ -46,12 +48,27 @@ my @Tests =
['no-nla2', '-d" "', {IN=>"1\na\n"}, {IN=>"2\nb"}, {OUT=>"1 2\na b\n"}],
['no-nla3', '-d" "', {IN=>"1\na"}, {IN=>"2\nb\n"}, {OUT=>"1 2\na b\n"}],
['no-nla4', '-d" "', {IN=>"1\na\n"}, {IN=>"2\nb\n"}, {OUT=>"1 2\na b\n"}],
+
+ # Specifying a delimiter with a trailing backslash would overrun a
+ # malloc'd buffer.
+ ['delim-bs1', q!-d'\'!, {IN=>{'a'x50=>''}}, {EXIT => 1},
+ # We print a single backslash into the expected output, so need four
+ # (two, each escaped) here.
+ {ERR => $msg . q!\\\\! . "\n"} ],
+
+ # Prior to coreutils-5.1.2, this sort of abuse would make paste
+ # scribble on command-line arguments. With paste from coreutils-5.1.0,
+ # this example would mangle the first file name argument, if it contains
+ # accepted backslash-escapes:
+ # $ paste -d\\ '123\b\b\b.....@' 2>&1 |cat -A
+ # paste: [EMAIL PROTECTED]@: No such file or directory$
+ ['delim-bs2', q!-d'\'!, {IN=>{'123\b\b\b.....@'=>''}}, {EXIT => 1},
+ {ERR => $msg . q!\\\\! . "\n"} ],
);
my $save_temps = $ENV{DEBUG};
my $verbose = $ENV{VERBOSE};
-my $prog = $ENV{PROG} || die "$0: \$PROG not specified in environment\n";
my $fail = run_tests ($program_name, $prog, [EMAIL PROTECTED], $save_temps,
$verbose);
exit $fail;
EOF
--
1.5.5.rc1.13.g79388
_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
