>From d6b43e5e1763bccf664e8afda4c4556ef5009d19 Mon Sep 17 00:00:00 2001 From: Assaf Gordon Date: Fri, 27 Jul 2018 01:56:26 -0600 Subject: [PATCH 1/2] sed: fix extraneous NUL in s///n command Under certain conditions sed would add an extraneous NUL: $ echo 0 | sed -e 's/$/a/2' | od -tx1 -An 30 00 0a This would happen when the regex is an empty (zero-length) match at the end of the line (e.g. '$' and 'a*$') and the substitute number flag ('n' in s///n) is higher than the number of actual matches (multiple EOL matches are possible with multiline match, e.g. 's/$/a/3m'). Details: The comment at the top of 'execute.c:do_subst()' says: /* The first part of the loop optimizes s/xxx// when xxx is at the start, and s/xxx$// */ Which refers to lines 1051-3: 1051 /* Copy stuff to the left of this match into the output string. */ 1052 if (start < offset) 1053 str_append(&s_accum, line.active + start, offset - start); The above code appends text to 's_accum' but does *not* update 'start'. Later on, if the s/// command includes 'n' flag, and if 'matched == 0' (an empty match), this comparison will be incorrect: 1081 if (start < line.length) 1082 matched = 1; Will in turn will set 'matched' to 1, and the 'str_append' call that follows (line 1087) will append an additional character. Because the empty match is EOL, the appended character is NUL. More examples that trigger the bug: echo 0 | sed -e 's/a*$/X/3' printf "%s\n" 0 0 0 | sed -e 'N;N;s/a*$/X/4m' Examples that do not trigger the bug: # The 'a*' empty regex matches at the beginning of the line (in # addition to the end of the line), and the optimization in line # 1052 is skipped. echo 0 | sed -e 's/a*/X/3' # There are 3 EOLs in the pattern space, s///3 is not too large. printf "%s\n" 0 0 0 | sed -e 'N;N;s/a*$/X/3m' This was discovered while investigating bug#32271 reported by address@hidden in https://lists.gnu.org/r/bug-sed/2018-07/msg00018.html . * NEWS: Mention the fix. * sed/execute.c (do_subst): Update 'start' as needed. * testsuite/bug-32271-1.sh: New test. * testsuite/local.mk (T): Add test. --- NEWS | 3 +++ sed/execute.c | 5 ++++- testsuite/bug32271-1.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++ testsuite/local.mk | 1 + 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100755 testsuite/bug32271-1.sh diff --git a/NEWS b/NEWS index ac166dd..f9293f3 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,9 @@ GNU sed NEWS -*- outline -*- sed no longer accesses invalid memory (heap overflow) when given invalid backreferences in 's' command [bug#32082, present at least since sed-4.0.6]. + sed no longer adds extraneous NUL when given s/$//n command. + [related to bug#32271, present since sed-4.0.7] + * Noteworthy changes in release 4.5 (2018-03-31) [stable] diff --git a/sed/execute.c b/sed/execute.c index a608f7d..f0a1e04 100644 --- a/sed/execute.c +++ b/sed/execute.c @@ -1050,7 +1050,10 @@ do_subst(struct subst *sub) /* Copy stuff to the left of this match into the output string. */ if (start < offset) - str_append(&s_accum, line.active + start, offset - start); + { + str_append(&s_accum, line.active + start, offset - start); + start = offset; + } /* If we're counting up to the Nth match, are we there yet? And even if we are there, there is another case we have to diff --git a/testsuite/bug32271-1.sh b/testsuite/bug32271-1.sh new file mode 100755 index 0000000..99a0934 --- /dev/null +++ b/testsuite/bug32271-1.sh @@ -0,0 +1,45 @@ +#!/bin/sh +# sed would incorrectly copy internal buffers under certain s/// uses. +# Before sed 4.6 these would result in an extraneous NUL at end of lines. +# + +# Copyright (C) 2018 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 . +. "${srcdir=.}/testsuite/init.sh"; path_prepend_ ./sed +print_ver_ sed + +printf '0\n' > in || framework_failure_ +printf '0\n' > exp || framework_failure_ + +# Before sed 4.6, this would result in: 0x30 0x00 0x0a. +sed -e 's/$/a/2' in > out 2> err || fail=1 + +compare exp out || fail=1 +compare /dev/null err || fail=1 + +# To ease debugging / error reporting (the above 'compare' +# will report "binary file differ" - not very helpful here) +if test -n "$fail" ; then + echo "---- TEST FAILED" + echo "out:" + od -tx1 out + echo "exp:" + od -tx1 exp + echo "err:" + od -tx1 err +fi + + +Exit $fail diff --git a/testsuite/local.mk b/testsuite/local.mk index bf4559f..1181a0c 100644 --- a/testsuite/local.mk +++ b/testsuite/local.mk @@ -44,6 +44,7 @@ LOG_COMPILER = false T = \ testsuite/misc.pl \ testsuite/bug32082.sh \ + testsuite/bug32271-1.sh \ testsuite/cmd-l.sh \ testsuite/cmd-R.sh \ testsuite/colon-with-no-label.sh \ -- 2.11.0