On 01/04/2018 05:38 PM, Michael Orlitzky wrote:
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -1427,6 +1427,13 @@ a command line argument is a symbolic link to a 
directory, traverse it.
  @cindex symbolic link to directory, traverse each that is encountered
  In a recursive traversal, traverse every symbolic link to a directory
  that is encountered.
+This option creates a security risk. In the presence of symlinks, the
+traversal is not guaranteed to be performed depth-first. As a result,
+there is a race condition: an attacker may be able to introduce a
+symlink at a point in the traversal that has yet to be reached. When
+it is reached, the operation will be performed on the target of that
+symlink, possibly allowing the attacker to escalate his privileges.
+
  @end macro
  @choptL

I'm not 100% happy with it yet.

* the patch adds the above to the macro choptL which is also used in
node chcon.  Do you see the danger for chcon(1), too?

* IMO we should avoid mentioning internal processing strategies like
"depth-first" - even guaranteeing depth-that would not avoid this
issue: there is no reason to trust FROM-USER more than NEW-USER.
Furthermore, not only these 2 users may be potential attackers in
this scenario, but also others, depending on the mode bits of the
involved files and directories, ACLs etc.

What about the attached?

Thanks & have a nice day,
Berny
>From 728090ba1413eef2b0fa2b727a337ebf9078c86d Mon Sep 17 00:00:00 2001
From: Michael Orlitzky <[email protected]>
Date: Thu, 4 Jan 2018 11:38:21 -0500
Subject: [PATCH] doc: warn about following symlinks recursively in chown/chgrp

In both chown and chgrp (which shares its code with chown), operating
on symlinks recursively has a window of vulnerability where the
destination user or group can change the target of the operation.
Warn about combining the --dereference, --recursive, and -L flags.

* doc/coreutils.texi (warnOptDerefWithRec): Add macro.
(node chown invocation): Add it to --dereference and -L.
(node chgrp invocation): Likewise.
---
 doc/coreutils.texi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 6bb9f0906..9f5f95b69 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -1428,6 +1428,19 @@ a command line argument is a symbolic link to a directory, traverse it.
 In a recursive traversal, traverse every symbolic link to a directory
 that is encountered.
 @end macro
+
+@c Append the following warning to -L where appropriate (e.g. chown).
+@macro warnOptDerefWithRec
+
+Combining this dereferencing option with the @option{--recursive} option
+may create a security risk:
+During the traversal of the directory tree, an attacker may be able to
+introduce a symlink to an arbitrary target; when the tool reaches that,
+the operation will be performed on the target of that symlink,
+possibly allowing the attacker to escalate privileges.
+
+@end macro
+
 @choptL
 
 @macro choptP
@@ -10995,6 +11008,7 @@ chown -h -R --from=OLDUSER NEWUSER /
 @findex lchown
 Do not act on symbolic links themselves but rather on what they point to.
 This is the default when not operating recursively.
+@warnOptDerefWithRec
 
 @item -h
 @itemx --no-dereference
@@ -11051,6 +11065,7 @@ Recursively change ownership of directories and their contents.
 @xref{Traversing symlinks}.
 
 @choptL
+@warnOptDerefWithRec
 @xref{Traversing symlinks}.
 
 @choptP
@@ -11125,6 +11140,7 @@ changed.
 @findex lchown
 Do not act on symbolic links themselves but rather on what they point to.
 This is the default when not operating recursively.
+@warnOptDerefWithRec
 
 @item -h
 @itemx --no-dereference
@@ -11180,6 +11196,7 @@ Recursively change the group ownership of directories and their contents.
 @xref{Traversing symlinks}.
 
 @choptL
+@warnOptDerefWithRec
 @xref{Traversing symlinks}.
 
 @choptP
-- 
2.15.1

Reply via email to