Re: md5 buffer underflow

[Jim Meyering]

Cristian Cadar <address@hidden> wrote:
>   Thanks again for the prompt confirmation of our previous bug reports.
> We found a new bug in md5sum, due to a buffer underflow.  Here is an
> example that seg faults on my machine:
>
>   $ md5sum -c -- md5sum_bug.txt
>   Segmentation fault
>
>   The file md5sum_bug.txt is attached to this message.
>
>   The bug (or at least the proximate cause) seems to be in function
> bsd_split_3 (md5sum.c:213):
>           i = s_len - 1;
>           while (i && s[i] != ')')
>               i--;
>
>   which when called with s_len=0, "underflows" string s.

Thanks yet again!
I've just pushed this patch:

        md5sum, sha1sum, etc: handle invalid input (i.e., don't segfault)
        * src/md5sum.c (bsd_split_3): Return right away if s_len == 0.
        * tests/misc/md5sum (bsd-segv): New test for the above.
        * tests/misc/sha1sum (bsd-segv): Likewise.
        * NEWS: Mention the bug fix.
        Reported by Cristian Cadar, Daniel Dunbar and Dawson Engler.

---
 NEWS               |    5 +++++
 src/md5sum.c       |    5 ++++-
 tests/misc/md5sum  |    9 +++++----
 tests/misc/sha1sum |    9 +++++----
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index e208b30..3a584e9 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,11 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   ls no longer segfaults on files in /proc when linked with an older version
   of libselinux.  E.g., ls -l /proc/sys would dereference a NULL pointer.

+  md5sum would segfault for invalid BSD-style input, e.g.,
+  echo 'MD5 (' | md5sum -c -  Now, md5sum ignores that line.
+  sha1sum, sha224sum, sha384sum, and sha512sum are affected, too.
+  [bug introduced in coreutils-5.1.0]
+
   "mkdir -Z x dir" no longer segfaults when diagnosing invalid context "x"
   mkfifo and mknod would fail similarly.  Now they're fixed.

diff --git a/src/md5sum.c b/src/md5sum.c
index 28bde99..5eb8494 100644
--- a/src/md5sum.c
+++ b/src/md5sum.c
@@ -1,5 +1,5 @@
 /* Compute MD5, SHA1, SHA224, SHA256, SHA384 or SHA512 checksum of files or 
strings
-   Copyright (C) 1995-2007 Free Software Foundation, Inc.
+   Copyright (C) 1995-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
@@ -205,6 +205,9 @@ bsd_split_3 (char *s, size_t s_len, unsigned char 
**hex_digest, char **file_name
 {
   size_t i;

+  if (s_len == 0)
+    return false;
+
   *file_name = s;

   /* Find end of filename. The BSD 'md5' and 'sha1' commands do not escape
diff --git a/tests/misc/md5sum b/tests/misc/md5sum
index ca23d94..25069fd 100755
--- a/tests/misc/md5sum
+++ b/tests/misc/md5sum
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Basic tests for "md5sum".

-# Copyright (C) 1998, 1999, 2003, 2005, 2007 Free Software Foundation, Inc.
+# Copyright (C) 1998-1999, 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
@@ -24,7 +24,7 @@ exec $PERL -w -I$srcdir/.. -MCoreutils -M"CuTmpdir qw($me)" 
-- - <<\EOF
 require 5.003;
 use strict;

-(my $program_name = $0) =~ s|.*/||;
+my $prog = 'md5sum';

 # Turn off localisation of executable's ouput.
 @ENV{qw(LANGUAGE LANG LC_ALL)} = ('C') x 3;
@@ -64,6 +64,8 @@ my @Tests =
      ['check-bsd3', '--check', '--status',
                                {IN=> {'f.md5' => "MD5 (f) = $degenerate\n"}},
                                {AUX=> {f=> 'bar'}}, {EXIT=> 1}],
+     ['bsd-segv', '--check', {IN=> {'z' => "MD5 ("}}, {EXIT=> 1},
+      {ERR=> "$prog: z: no properly formatted MD5 checksum lines found\n"}],
     );

 # Insert the `--text' argument for each test.
@@ -76,7 +78,6 @@ foreach $t (@Tests)
 my $save_temps = $ENV{DEBUG};
 my $verbose = $ENV{VERBOSE};

-my $prog = 'md5sum';
-my $fail = run_tests ($program_name, $prog, address@hidden, $save_temps, 
$verbose);
+my $fail = run_tests ($prog, $prog, address@hidden, $save_temps, $verbose);
 exit $fail;
 EOF
diff --git a/tests/misc/sha1sum b/tests/misc/sha1sum
index b7d288f..452236d 100755
--- a/tests/misc/sha1sum
+++ b/tests/misc/sha1sum
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Test "sha1sum".

-# Copyright (C) 2000, 2003, 2005-2007 Free Software Foundation, Inc.
+# Copyright (C) 2000, 2003, 2005-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
@@ -24,7 +24,7 @@ exec $PERL -w -I$srcdir/.. -MCoreutils -M"CuTmpdir qw($me)" 
-- - <<\EOF
 require 5.003;
 use strict;

-(my $program_name = $0) =~ s|.*/||;
+my $prog = 'sha1sum';

 # Turn off localisation of executable's ouput.
 @ENV{qw(LANGUAGE LANG LC_ALL)} = ('C') x 3;
@@ -66,6 +66,8 @@ my @Tests =
      ['check-bsd3', '--check', '--status',
                        {IN=> {'f.sha1' => "SHA1 (f) = $sha_degenerate\n"}},
                        {AUX=> {f=> 'bar'}}, {EXIT=> 1}],
+     ['bsd-segv', '--check', {IN=> {'z' => "SHA1 ("}}, {EXIT=> 1},
+      {ERR=> "$prog: z: no properly formatted SHA1 checksum lines found\n"}],
     );

 # Insert the `--text' argument for each test.
@@ -78,7 +80,6 @@ foreach $t (@Tests)
 my $save_temps = $ENV{DEBUG};
 my $verbose = $ENV{VERBOSE};

-my $prog = 'sha1sum';
-my $fail = run_tests ($program_name, $prog, address@hidden, $save_temps, 
$verbose);
+my $fail = run_tests ($prog, $prog, address@hidden, $save_temps, $verbose);
 exit $fail;
 EOF
--
1.5.5.50.gab781