GNU bug report logs - #39165
[PATCH] sed: handle very long execution lines

Previous Next

To reply to this bug, email your comments to 39165 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tobias Stoeckmann <tobias <at> stoeckmann.org>
To: bug-sed <at> gnu.org
Subject: [PATCH] sed: handle very long execution lines
Date: Fri, 17 Jan 2020 20:49:33 +0100

If sed is called with an excessively long execution line, then it is
prone to an out of boundary memory access.

The problem is that the length of the execution line, which is a
size_t, is temporarily stored in an int. This means that on systems
which have a 64 bit size_t and a 32 bit int (e.g. linux amd64) an
execution line which exceeds 2 GB will overflow int. If it is just
slightly larger than 2 GB, the negative int value is used as an
array index to finish the execution line string with '\0' which
therefore triggers the out of boundary access.

This problem is probably never triggered in reality, but can be
provoked like this (given that 'e' support is compiled in):

$ dd if=/dev/zero bs=1M count=2049 | tr '\0' 'e' > e-command.txt
$ sed -f e-command.txt /etc/fstab
Segmentation fault (core dumped)
$ _

While at it, I also adjusted another int/size_t conversion, even
though it is a purely cosmetical change, because it can never be
larger than 4096.

Signed-off-by: Tobias Stoeckmann <tobias <at> stoeckmann.org>
---
 sed/execute.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sed/execute.c b/sed/execute.c
index c5f07cc..8f43f2e 100644
--- a/sed/execute.c
+++ b/sed/execute.c
@@ -1347,7 +1347,7 @@ execute_program (struct vector *vec, struct input *input)
               panic (_("`e' command not supported"));
 #else
               FILE *pipe_fp;
-              int cmd_length = cur_cmd->x.cmd_txt.text_length;
+              size_t cmd_length = cur_cmd->x.cmd_txt.text_length;
               line_reset (&s_accum, NULL);
 
               if (!cmd_length)
@@ -1367,7 +1367,7 @@ execute_program (struct vector *vec, struct input *input)
 
               {
                 char buf[4096];
-                int n;
+                size_t n;
                 while (!feof (pipe_fp))
                   if ((n = fread (buf, sizeof (char), 4096, pipe_fp)) > 0)
                     {
-- 
2.25.0

Message #8 received at 39165 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Tobias Stoeckmann <tobias <at> stoeckmann.org>
Cc: 39165 <at> debbugs.gnu.org
Subject: Re: bug#39165: [PATCH] sed: handle very long execution lines
Date: Fri, 17 Jan 2020 18:27:20 -0800

[Message part 1 (text/plain, inline)]

On Fri, Jan 17, 2020 at 1:11 PM Tobias Stoeckmann <tobias <at> stoeckmann.org> wrote:
> If sed is called with an excessively long execution line, then it is
> prone to an out of boundary memory access.
...

Good find and patch. Thank you!
I've adjusted your commit log slightly.

The "tiny change" note indicates that your diff is small enough that
you do not have to assign copyright. I have also made minor wording
changes, added the standard ChangeLog-style filename (function) lines
and added the obligatory bug-fix reference in NEWS.

We could add a test for this and even label it "expensive" or
"very-expensive", but so far I don't think it's worthwhile.

Will push the attached tomorrow.

[sed-2G-execute-line-segv.diff (application/octet-stream, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.