Subject: Re: bug#22127: Segfault / null pointer access in function str_append_modified()

From: Jim Meyering <[email-masked]>
Date: Thu, 17 Dec 2015 18:56:51 -0800

On Wed, Dec 9, 2015 at 3:42 AM, Hanno Böck <[email-masked]> wrote:
> Hi,
>
> With a malformed input (see attachmend) sed can crash in the function
> str_append_modified()
>
> Test:
> echo|./sed -f sed-nullptr-str_append_modified
>
> Seems to be a null pointer access.
> This only seems to happen in the git code of sed and not in 4.2.2.
>
> This is the stack trace from address sanitizer:
> ==21489==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fd77e298c16 bp 0x611000009c86 sp 0x7fffe46649d0 T0)
>     #0 0x7fd77e298c15 in wcrtomb /var/tmp/portage/sys-libs/glibc-2.22-r1/work/glibc-2.22/wcsmbs/wcrtomb.c:89
>     #1 0x5029ca in str_append_modified /mnt/ram/sed-plain/sed/execute.c:273:11
>     #2 0x4faa8c in append_replacement /mnt/ram/sed-plain/sed/execute.c:992:11
>     #3 0x4faa8c in do_subst /mnt/ram/sed-plain/sed/execute.c:1078
>     #4 0x4faa8c in execute_program /mnt/ram/sed-plain/sed/execute.c:1513
>     #5 0x4faa8c in process_files /mnt/ram/sed-plain/sed/execute.c:1681
>     #6 0x4e1365 in main /mnt/ram/sed-plain/sed/sed.c:362:17
>     #7 0x7fd77e21b62f in __libc_start_main /var/tmp/portage/sys-libs/glibc-2.22-r1/work/glibc-2.22/csu/libc-start.c:289
>     #8 0x4191a8 in _start (/tmp/sed+0x4191a8)
>
>
> This was found with the help of american fuzzy lop.

Thank you for the report.
I've reduced it to the following one-liner (demonstrating
failure with an ASAN-enabled binary), and have attached
a patch:

$ echo > k; LC_ALL=en_US.utf8 sed/sed $(printf 's/^/\\L\233\375\134\200/') k
=================================================================
==3335==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x60600000edb2 at pc 0x000000446933 bp 0x7ffd73a42ee0 sp
0x7ffd73a42690
WRITE of size 6 at 0x60600000edb2 thread T0
    #0 0x446932 in __interceptor_wcrtomb
../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:2751
    #1 0x4dc393 in str_append_modified /home/j/w/co/sed/sed/execute.c:273
    #2 0x4e08e2 in append_replacement /home/j/w/co/sed/sed/execute.c:992
    #3 0x4e1272 in do_subst /home/j/w/co/sed/sed/execute.c:1078
    #4 0x4e2d09 in execute_program /home/j/w/co/sed/sed/execute.c:1513
    #5 0x4e359a in process_files /home/j/w/co/sed/sed/execute.c:1681
    #6 0x4d446a in main /home/j/w/co/sed/sed/sed.c:362
    #7 0x7f5541c7f57f in __libc_start_main (/lib64/libc.so.6+0x2057f)
    #8 0x406d18 in _start (/home/j/w/co/sed/sed/sed+0x406d18)

0x60600000edb2 is located 0 bytes to the right of 50-byte region
[0x60600000ed80,0x60600000edb2)
allocated by thread T0 here:
    #0 0x4a2050 in __interceptor_calloc
../../../../libsanitizer/asan/asan_malloc_linux.cc:54
    #1 0x4e59d3 in ck_malloc /home/j/w/co/sed/sed/utils.c:398
    #2 0x4dc4e9 in line_init /home/j/w/co/sed/sed/execute.c:288
    #3 0x4dc75f in line_reset /home/j/w/co/sed/sed/execute.c:306
    #4 0x4e0d37 in do_subst /home/j/w/co/sed/sed/execute.c:1023
    #5 0x4e2d09 in execute_program /home/j/w/co/sed/sed/execute.c:1513
    #6 0x4e359a in process_files /home/j/w/co/sed/sed/execute.c:1681
    #7 0x4d446a in main /home/j/w/co/sed/sed/sed.c:362
    #8 0x7f5541c7f57f in __libc_start_main (/lib64/libc.so.6+0x2057f)

SUMMARY: AddressSanitizer: heap-buffer-overflow
../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:2751
in __interceptor_wcrtomb
Shadow bytes around the buggy address:
  0x0c0c7fff9d60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff9d70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff9d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff9d90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff9da0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c0c7fff9db0: 00 00 00 00 00 00[02]fa fa fa fa fa 00 00 00 00
  0x0c0c7fff9dc0: 00 00 02 fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c7fff9dd0: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c0c7fff9de0: 00 00 00 00 00 00 00 00 fa fa fa fa fd fd fd fd
  0x0c0c7fff9df0: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fa
  0x0c0c7fff9e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
From 7361146db70d7618943cba5722045f49ac13ac40 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Fri, 11 Dec 2015 21:31:29 -0800
Subject: [PATCH] sed: fix a heap-clobbering buffer overrun
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* sed/execute.c (str_append_modified): Upon encountering an
incomplete multi-byte sequence in an substitution replacement
string do not treat mbrtowc's return value of -2 as a large,
positive number.
* testsuite/subst-mb-incomplete.sh: New file/test.
* testsuite/Makefile.am (T): Add it.
Reported by Hanno Böck in http://debbugs.gnu.org/22127.
Hanno used the AFL fuzzer to find the segfault-inducing input
from which I derived the test's input.
Introduced by v4.2.2-76-g78e8e58, this bug was never in a release.
---
 sed/execute.c                    |  8 ++++----
 testsuite/Makefile.am            |  1 +
 testsuite/subst-mb-incomplete.sh | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 4 deletions(-)
 create mode 100755 testsuite/subst-mb-incomplete.sh

diff --git a/sed/execute.c b/sed/execute.c
index 7bc394a..5bebfc2 100644
--- a/sed/execute.c
+++ b/sed/execute.c
@@ -231,15 +231,15 @@ str_append_modified(struct line *to, const char *string, size_t length,
           continue;
         }

-      if (n > 0)
-        string += n, length -= n;
-      else
+      if (n == 0 || n == (size_t) -2)
         {
-          /* Incomplete sequence, copy it manually.  */
+          /* L'\0' or an incomplete sequence: copy it manually.  */
           str_append(to, string, length);
           return;
         }

+      string += n, length -= n;
+
       /* Convert the first character specially... */
       if (type & (REPL_UPPERCASE_FIRST | REPL_LOWERCASE_FIRST))
         {
diff --git a/testsuite/Makefile.am b/testsuite/Makefile.am
index 0c065ed..b37e402 100644
--- a/testsuite/Makefile.am
+++ b/testsuite/Makefile.am
@@ -12,6 +12,7 @@ T =				\
   in-place-hyphen.sh		\
   invalid-mb-seq-UMR.sh		\
   range-overlap.sh		\
+  subst-mb-incomplete.sh	\
   temp-file-cleanup.sh

 TESTS = $(check_PROGRAMS) $(SEDTESTS) $(T)
diff --git a/testsuite/subst-mb-incomplete.sh b/testsuite/subst-mb-incomplete.sh
new file mode 100755
index 0000000..41c6e13
--- /dev/null
+++ b/testsuite/subst-mb-incomplete.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+# Ensure that sed no longer writes beyond the end of a heap buffer when
+# performing a substitution with a replacement string containing an
+# incomplete multi-byte character.
+
+# Copyright (C) 2015 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_ ../sed
+print_ver_ sed
+
+require_en_utf8_locale_
+
+echo > in || framework_failure_
+printf '\233\375\200\n' > exp-out || framework_failure_
+
+fail=0
+LC_ALL=en_US.utf8 sed $(printf 's/^/\\L\233\375\\\200/') in > out 2> err
+
+compare exp-out out || fail=1
+compare /dev/null err || fail=1
+
+Exit $fail
-- 
2.6.2