GNU bug report logs - #20796
[PATCH] Document behavior of sed -i -

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 20796 in the body.
You can then email your comments to 20796 AT debbugs.gnu.org in the normal way.

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

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

From: Stanislav Brabec <sbrabec <at> suse.com>
To: bug-sed <at> gnu.org
Subject: [PATCH] Document behavior of sed -i -
Date: Fri, 12 Jun 2015 17:32:50 +0200

"sed -i -" does not fail, but it also does not do what one would expect.
Document it, as it could have security implications:

Example:
The sed command below looks broken, but it is executed and succeeds:

ln -s /etc/passwd -- -
echo root | sed -i --follow-symlinks s/root/parrot/ -

Signed-off-by: Stanislav Brabec <sbrabec <at> suse.cz>
---
 doc/sed-in.texi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/sed-in.texi b/doc/sed-in.texi
index 0e10cde..c8f1289 100644
--- a/doc/sed-in.texi
+++ b/doc/sed-in.texi
@@ -180,6 +180,7 @@ sed OPTIONS... [SCRIPT] [INPUTFILE...]
 @end example
 
 If you do not specify @var{INPUTFILE}, or if @var{INPUTFILE} is @file{-},
+and @option{-i} is not used,
 @command{sed} filters the contents of the standard input.  The @var{script}
 is actually the first non-option parameter, which @command{sed} specially
 considers a script and not an input file if (and only if) none of the
-- 
2.4.2

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec <at> suse.com
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

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

From: Jim Meyering <jim <at> meyering.net>
To: Stanislav Brabec <sbrabec <at> suse.com>
Cc: 20796 <at> debbugs.gnu.org
Subject: Re: bug#20796: [PATCH] Document behavior of sed -i -
Date: Sat, 20 Jun 2015 20:01:20 -0700

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

On Fri, Jun 12, 2015 at 8:32 AM, Stanislav Brabec <sbrabec <at> suse.com> wrote:
> "sed -i -" does not fail, but it also does not do what one would expect.
> Document it, as it could have security implications:
>
> Example:
> The sed command below looks broken, but it is executed and succeeds:
>
> ln -s /etc/passwd -- -
> echo root | sed -i --follow-symlinks s/root/parrot/ -
>
> Signed-off-by: Stanislav Brabec <sbrabec <at> suse.cz>
> ---
>  doc/sed-in.texi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/doc/sed-in.texi b/doc/sed-in.texi
> index 0e10cde..c8f1289 100644
> --- a/doc/sed-in.texi
> +++ b/doc/sed-in.texi
> @@ -180,6 +180,7 @@ sed OPTIONS... [SCRIPT] [INPUTFILE...]
>  @end example
>
>  If you do not specify @var{INPUTFILE}, or if @var{INPUTFILE} is @file{-},
> +and @option{-i} is not used,
>  @command{sed} filters the contents of the standard input.  The @var{script}
>  is actually the first non-option parameter, which @command{sed} specially
>  considers a script and not an input file if (and only if) none of the

Thank you for the patch.
However, rather than documenting this surprising behavior,
I propose to remove the anomaly altogether with the attached patch.
Does anyone see a reason to retain the behavior of treating "-"
like "./-"?

[0001-sed-i-do-not-treat-as-a-file-name.patch (application/octet-stream, attachment)]

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

From: Stanislav Brabec <sbrabec <at> suse.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 20796 <at> debbugs.gnu.org
Subject: Re: bug#20796: [PATCH] Document behavior of sed -i -
Date: Mon, 22 Jun 2015 15:43:26 +0200

Jim Meyering wrote:

> However, rather than documenting this surprising behavior,
> I propose to remove the anomaly altogether with the attached patch.
> Does anyone see a reason to retain the behavior of treating "-"
> like "./-"?
>
The reason is simple: "sed -i" makes no sense on stdin, so the special 
handling od "-" is disabled.

We can either ignore "-i" for stdin, or report error:
"Editing standard input in place is an undefined operation."

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec <at> suse.com
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

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

From: Jim Meyering <jim <at> meyering.net>
To: Stanislav Brabec <sbrabec <at> suse.com>
Cc: 20796 <20796 <at> debbugs.gnu.org>
Subject: Re: bug#20796: [PATCH] Document behavior of sed -i -
Date: Mon, 22 Jun 2015 07:06:46 -0700

On Mon, Jun 22, 2015 at 6:43 AM, Stanislav Brabec <sbrabec <at> suse.com> wrote:
> Jim Meyering wrote:
>
>> However, rather than documenting this surprising behavior,
>> I propose to remove the anomaly altogether with the attached patch.
>> Does anyone see a reason to retain the behavior of treating "-"
>> like "./-"?
>>
> The reason is simple: "sed -i" makes no sense on stdin, so the special
> handling od "-" is disabled.
>
> We can either ignore "-i" for stdin, or report error:
> "Editing standard input in place is an undefined operation."

Ignoring a fundamental user-specified option like -i is not an option,
so I chose the latter: to make this nonsensical usage evoke
a diagnostic:

  $ sed -i s/a/b/ -
  sed: couldn't edit -: is a terminal

I chose to use the same diagnostic that was already printed for
a case like "sed -i s/a/b/ /dev/stdin".  It feels slightly better to
include the name of the offending device.

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

From: Stanislav Brabec <sbrabec <at> suse.com>
To: Jim Meyering <jim <at> meyering.net>, sbrabec <at> suse.cz
Cc: 20796 <20796 <at> debbugs.gnu.org>
Subject: [PATCH] Keep special meaning of "-" even for "sed -i"
Date: Mon, 22 Jun 2015 16:40:31 +0200

"sed -i -" does not fail, but it also does not do what one would expect.
It attempts to access file "./-" and edit it in place.

Keep special meaning of "-" even for "sed -i". It will cause failure,
which is what most people would expect:

sed: couldn't edit /dev/stdin: is a terminal

Note that the previous behavior could have security implications:
ln -s /etc/passwd -- -
echo root | sed -i --follow-symlinks s/root/parrot/ -

Signed-off-by: Stanislav Brabec <sbrabec <at> suse.com>
---
 sed/execute.c        |  2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sed/execute.c b/sed/execute.c
index bfde525..2cba844 100644
--- a/sed/execute.c
+++ b/sed/execute.c
@@ -550,7 +550,7 @@ open_next_file(const char *name, struct input *input)
   buffer.length = 0;
 
   input->in_file_name = name;
-  if (name[0] == '-' && name[1] == '\0' && !in_place_extension)
+  if (name[0] == '-' && name[1] == '\0')
     {
       clearerr(stdin);	/* clear any stale EOF indication */
 #if defined WIN32 || defined _WIN32 || defined __CYGWIN__ \
-- 
2.4.3

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec <at> suse.com
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

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

From: Jim Meyering <jim <at> meyering.net>
To: Stanislav Brabec <sbrabec <at> suse.com>
Cc: sbrabec <at> suse.cz, 20796 <20796 <at> debbugs.gnu.org>
Subject: Re: [PATCH] Keep special meaning of "-" even for "sed -i"
Date: Mon, 22 Jun 2015 08:03:52 -0700

On Mon, Jun 22, 2015 at 7:40 AM, Stanislav Brabec <sbrabec <at> suse.com> wrote:
> "sed -i -" does not fail, but it also does not do what one would expect.
> It attempts to access file "./-" and edit it in place.
>
> Keep special meaning of "-" even for "sed -i". It will cause failure,
> which is what most people would expect:
>
> sed: couldn't edit /dev/stdin: is a terminal
>
> Note that the previous behavior could have security implications:
> ln -s /etc/passwd -- -
> echo root | sed -i --follow-symlinks s/root/parrot/ -
>
> Signed-off-by: Stanislav Brabec <sbrabec <at> suse.com>
> ---
>  sed/execute.c        |  2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sed/execute.c b/sed/execute.c

Thanks for the patch.
I guess you didn't see that I posted one yesterday?

  http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20796#8

Message #25 received at 20796-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Stanislav Brabec <sbrabec <at> suse.com>
Cc: 20796-done <at> debbugs.gnu.org
Subject: Re: bug#20796: [PATCH] Document behavior of sed -i -
Date: Tue, 23 Jun 2015 09:11:10 -0700

On Mon, Jun 22, 2015 at 7:06 AM, Jim Meyering <jim <at> meyering.net> wrote:
> On Mon, Jun 22, 2015 at 6:43 AM, Stanislav Brabec <sbrabec <at> suse.com> wrote:
>> Jim Meyering wrote:
>>
>>> However, rather than documenting this surprising behavior,
>>> I propose to remove the anomaly altogether with the attached patch.
>>> Does anyone see a reason to retain the behavior of treating "-"
>>> like "./-"?
>>>
>> The reason is simple: "sed -i" makes no sense on stdin, so the special
>> handling od "-" is disabled.
>>
>> We can either ignore "-i" for stdin, or report error:
>> "Editing standard input in place is an undefined operation."
>
> Ignoring a fundamental user-specified option like -i is not an option,
> so I chose the latter: to make this nonsensical usage evoke
> a diagnostic:
>
>   $ sed -i s/a/b/ -
>   sed: couldn't edit -: is a terminal
>
> I chose to use the same diagnostic that was already printed for
> a case like "sed -i s/a/b/ /dev/stdin".  It feels slightly better to
> include the name of the offending device.

I've pushed that change.

Previous Next

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