path: root/main/augeas/CVE-2012-0787.patch

From db42d52a8a1842726eb88e8da326ea37c2edaa27 Mon Sep 17 00:00:00 2001
From: Dominic Cleal <dcleal@redhat.com>
Date: Sun, 25 Mar 2012 17:07:54 +0100
Subject: [PATCH] Prevent cross-mountpoint attacks via .augsave during saving

Previously Augeas would open PATH.augsave for writing if a rename from PATH to
PATH.augsave failed, then write the file contents in.  Now if the rename fails,
it tries to unlink PATH.augsave and open it with O_EXCL first.

Mountpoints remain permitted at either PATH or PATH.augnew provided
/augeas/save/copy_if_rename_fails exists.

* src/transform.c (clone_file):
    add argument to perform unlink and O_EXCL on destination filename after a
    rename failure to prevent PATH.augsave being a mountpoint
* src/transform.c (transform_save, remove_file):
    always try to unlink PATH.augsave if rename fails, only allowing PATH to be
    a mountpoint; allow PATH or PATH.augnew to be mountpoints
* tests/
    test-put-mount: check PATH being a mountpoint is supported
    test-put-mount-augnew.sh: check PATH.augnew being a mountpoint is supported
    test-put-mount-augsave.sh: check unlink error when PATH.augsave is a mount

Fixes BZ 772261
(cherry picked from commit b8de6a8c5cfffb007149036ffa561ced4d11c462)
---
 src/transform.c                 | 40 ++++++++++++++++++++----
 tests/Makefile.am               |  5 +--
 tests/test-put-mount-augnew.sh  | 69 +++++++++++++++++++++++++++++++++++++++++
 tests/test-put-mount-augsave.sh | 62 ++++++++++++++++++++++++++++++++++++
 tests/test-put-mount.sh         | 55 ++++++++++++++++++++++++++++++++
 5 files changed, 223 insertions(+), 8 deletions(-)
 create mode 100755 tests/test-put-mount-augnew.sh
 create mode 100755 tests/test-put-mount-augsave.sh
 create mode 100755 tests/test-put-mount.sh

diff --git a/src/transform.c b/src/transform.c
index 0936eb7..00ca3ec 100644
--- a/src/transform.c
+++ b/src/transform.c
@@ -27,6 +27,7 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <fcntl.h>
 #include <unistd.h>
 #include <selinux/selinux.h>
 #include <stdbool.h>
@@ -784,14 +785,21 @@ static int transfer_file_attrs(FILE *from, FILE *to,
  * means that FROM or TO is a bindmounted file), and COPY_IF_RENAME_FAILS
  * is true, copy the contents of FROM into TO and delete FROM.
  *
+ * If COPY_IF_RENAME_FAILS and UNLINK_IF_RENAME_FAILS are true, and the above
+ * copy mechanism is used, it will unlink the TO path and open with O_EXCL
+ * to ensure we only copy *from* a bind mount rather than into an attacker's
+ * mount placed at TO (e.g. for .augsave).
+ *
  * Return 0 on success (either rename succeeded or we copied the contents
  * over successfully), -1 on failure.
  */
 static int clone_file(const char *from, const char *to,
-                      const char **err_status, int copy_if_rename_fails) {
+                      const char **err_status, int copy_if_rename_fails,
+                      int unlink_if_rename_fails) {
     FILE *from_fp = NULL, *to_fp = NULL;
     char buf[BUFSIZ];
     size_t len;
+    int to_fd = -1, to_oflags, r;
     int result = -1;
 
     if (rename(from, to) == 0)
@@ -808,10 +816,23 @@ static int clone_file(const char *from, const char *to,
         goto done;
     }
 
-    if (!(to_fp = fopen(to, "w"))) {
+    if (unlink_if_rename_fails) {
+        r = unlink(to);
+        if (r < 0) {
+            *err_status = "clone_unlink_dst";
+            goto done;
+        }
+    }
+
+    to_oflags = unlink_if_rename_fails ? O_EXCL : O_TRUNC;
+    if ((to_fd = open(to, O_WRONLY|O_CREAT|to_oflags, S_IRUSR|S_IWUSR)) < 0) {
         *err_status = "clone_open_dst";
         goto done;
     }
+    if (!(to_fp = fdopen(to_fd, "w"))) {
+        *err_status = "clone_fdopen_dst";
+        goto done;
+    }
 
     if (transfer_file_attrs(from_fp, to_fp, err_status) < 0)
         goto done;
@@ -838,8 +859,15 @@ static int clone_file(const char *from, const char *to,
  done:
     if (from_fp != NULL)
         fclose(from_fp);
-    if (to_fp != NULL && fclose(to_fp) != 0)
+    if (to_fp != NULL) {
+        if (fclose(to_fp) != 0) {
+            *err_status = "clone_fclose_dst";
+            result = -1;
+        }
+    } else if (to_fd >= 0 && close(to_fd) < 0) {
+        *err_status = "clone_close_dst";
         result = -1;
+    }
     if (result != 0)
         unlink(to);
     if (result == 0)
@@ -1072,7 +1100,7 @@ int transform_save(struct augeas *aug, struct tree *xfm,
                 goto done;
             }
 
-            r = clone_file(augorig_canon, augsave, &err_status, 1);
+            r = clone_file(augorig_canon, augsave, &err_status, 1, 1);
             if (r != 0) {
                 dyn_err_status = strappend(err_status, "_augsave");
                 goto done;
@@ -1080,7 +1108,7 @@ int transform_save(struct augeas *aug, struct tree *xfm,
         }
     }
 
-    r = clone_file(augtemp, augdest, &err_status, copy_if_rename_fails);
+    r = clone_file(augtemp, augdest, &err_status, copy_if_rename_fails, 0);
     if (r != 0) {
         dyn_err_status = strappend(err_status, "_augtemp");
         goto done;
@@ -1171,7 +1199,7 @@ int remove_file(struct augeas *aug, struct tree *tree) {
                 goto error;
         }
 
-        r = clone_file(augorig_canon, augsave, &err_status, 1);
+        r = clone_file(augorig_canon, augsave, &err_status, 1, 1);
         if (r != 0) {
             dyn_err_status = strappend(err_status, "_augsave");
             goto error;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3af1a0e..69c3c89 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -152,8 +152,9 @@ check_SCRIPTS = \
   $(lens_tests) \
   test-get.sh test-augtool.sh \
   test-put-symlink.sh test-put-symlink-augnew.sh \
-  test-put-symlink-augsave.sh test-put-symlink-augtemp.sh test-save-empty.sh \
-  test-bug-1.sh test-idempotent.sh test-preserve.sh \
+  test-put-symlink-augsave.sh test-put-symlink-augtemp.sh \
+  test-put-mount.sh test-put-mount-augnew.sh test-put-mount-augsave.sh \
+  test-save-empty.sh test-bug-1.sh test-idempotent.sh test-preserve.sh \
   test-events-saved.sh test-save-mode.sh test-unlink-error.sh \
   test-augtool-empty-line.sh test-augtool-modify-root.sh
 
diff --git a/tests/test-put-mount-augnew.sh b/tests/test-put-mount-augnew.sh
new file mode 100755
index 0000000..cb95bda
--- /dev/null
+++ b/tests/test-put-mount-augnew.sh
@@ -0,0 +1,69 @@
+#! /bin/bash
+
+# Test that we can write into a bind mount placed at PATH.augnew with the
+# copy_if_rename_fails flag.
+# This requires that EXDEV or EBUSY is returned from rename(2) to activate the
+# code path, so set up a bind mount on Linux.
+
+if [ $UID -ne 0 -o "$(uname -s)" != "Linux" ]; then
+    echo "Test can only be run as root on Linux to create bind mounts"
+    exit 77
+fi
+
+ROOT=$abs_top_builddir/build/test-put-mount-augnew
+LENSES=$abs_top_srcdir/lenses
+
+HOSTS=$ROOT/etc/hosts
+HOSTS_AUGNEW=${HOSTS}.augnew
+TARGET=$ROOT/other/real_hosts
+
+rm -rf $ROOT
+mkdir -p $(dirname $HOSTS)
+mkdir -p $(dirname $TARGET)
+
+echo 127.0.0.1 localhost > $HOSTS
+touch $TARGET $HOSTS_AUGNEW
+
+mount --bind $TARGET $HOSTS_AUGNEW
+Exit() {
+    umount $HOSTS_AUGNEW
+    exit $1
+}
+
+HOSTS_SUM=$(sum $HOSTS)
+
+augtool --nostdinc -I $LENSES -r $ROOT --new <<EOF
+set /augeas/save/copy_if_rename_fails 1
+set /files/etc/hosts/1/alias myhost
+save
+print /augeas//error
+EOF
+
+if [ ! -f $HOSTS ] ; then
+    echo "/etc/hosts is no longer a regular file"
+    Exit 1
+fi
+if [ ! "x${HOSTS_SUM}" = "x$(sum $HOSTS)" ]; then
+    echo "/etc/hosts has changed"
+    Exit 1
+fi
+if [ ! "x${HOSTS_SUM}" = "x$(sum $HOSTS)" ]; then
+    echo "/etc/hosts has changed"
+    Exit 1
+fi
+
+if [ ! -s $HOSTS_AUGNEW ]; then
+    echo "/etc/hosts.augnew is empty"
+    Exit 1
+fi
+if [ ! -s $TARGET ]; then
+    echo "/other/real_hosts is empty"
+    Exit 1
+fi
+
+if ! grep myhost $TARGET >/dev/null; then
+    echo "/other/real_hosts does not contain the modification"
+    Exit 1
+fi
+
+Exit 0
diff --git a/tests/test-put-mount-augsave.sh b/tests/test-put-mount-augsave.sh
new file mode 100755
index 0000000..31fcfca
--- /dev/null
+++ b/tests/test-put-mount-augsave.sh
@@ -0,0 +1,62 @@
+#! /bin/bash
+
+# Test that we don't follow bind mounts when writing to .augsave.
+# This requires that EXDEV or EBUSY is returned from rename(2) to activate the
+# code path, so set up a bind mount on Linux.
+
+if [ $UID -ne 0 -o "$(uname -s)" != "Linux" ]; then
+    echo "Test can only be run as root on Linux to create bind mounts"
+    exit 77
+fi
+
+actual() {
+    (augtool --nostdinc -I $LENSES -r $ROOT --backup | grep ^/augeas) <<EOF
+    set /augeas/save/copy_if_rename_fails 1
+    set /files/etc/hosts/1/alias myhost
+    save
+    print /augeas//error
+EOF
+}
+
+expected() {
+    cat <<EOF
+/augeas/files/etc/hosts/error = "clone_unlink_dst_augsave"
+/augeas/files/etc/hosts/error/message = "Device or resource busy"
+EOF
+}
+
+ROOT=$abs_top_builddir/build/test-put-mount-augsave
+LENSES=$abs_top_srcdir/lenses
+
+HOSTS=$ROOT/etc/hosts
+HOSTS_AUGSAVE=${HOSTS}.augsave
+
+ATTACK_FILE=$ROOT/other/attack
+
+rm -rf $ROOT
+mkdir -p $(dirname $HOSTS)
+mkdir -p $(dirname $ATTACK_FILE)
+
+echo 127.0.0.1 localhost > $HOSTS
+touch $ATTACK_FILE $HOSTS_AUGSAVE
+
+mount --bind $ATTACK_FILE $HOSTS_AUGSAVE
+Exit() {
+    umount $HOSTS_AUGSAVE
+    exit $1
+}
+
+ACTUAL=$(actual)
+EXPECTED=$(expected)
+if [ "$ACTUAL" != "$EXPECTED" ]; then
+    echo "No error when trying to unlink augsave (a bind mount):"
+    echo "$ACTUAL"
+    exit 1
+fi
+
+if [ -s $ATTACK_FILE ]; then
+    echo "/other/attack now contains data, should be blank"
+    Exit 1
+fi
+
+Exit 0
diff --git a/tests/test-put-mount.sh b/tests/test-put-mount.sh
new file mode 100755
index 0000000..210bc10
--- /dev/null
+++ b/tests/test-put-mount.sh
@@ -0,0 +1,55 @@
+#! /bin/bash
+
+# Test that we can write into a bind mount with the copy_if_rename_fails flag.
+# This requires that EXDEV or EBUSY is returned from rename(2) to activate the
+# code path, so set up a bind mount on Linux.
+
+if [ $UID -ne 0 -o "$(uname -s)" != "Linux" ]; then
+    echo "Test can only be run as root on Linux to create bind mounts"
+    exit 77
+fi
+
+ROOT=$abs_top_builddir/build/test-put-mount
+LENSES=$abs_top_srcdir/lenses
+
+HOSTS=$ROOT/etc/hosts
+TARGET=$ROOT/other/real_hosts
+
+rm -rf $ROOT
+mkdir -p $(dirname $HOSTS)
+mkdir -p $(dirname $TARGET)
+
+echo 127.0.0.1 localhost > $TARGET
+touch $HOSTS
+
+mount --bind $TARGET $HOSTS
+Exit() {
+    umount $HOSTS
+    exit $1
+}
+
+HOSTS_SUM=$(sum $HOSTS)
+
+augtool --nostdinc -I $LENSES -r $ROOT <<EOF
+set /augeas/save/copy_if_rename_fails 1
+set /files/etc/hosts/1/alias myhost
+save
+print /augeas//error
+EOF
+
+if [ ! "x${HOSTS_SUM}" != "x$(sum $HOSTS)" ]; then
+    echo "/etc/hosts hasn't changed"
+    Exit 1
+fi
+
+if [ ! "x${HOSTS_SUM}" != "x$(sum $TARGET)" ]; then
+    echo "/other/real_hosts hasn't changed"
+    Exit 1
+fi
+
+if ! grep myhost $TARGET >/dev/null; then
+    echo "/other/real_hosts does not contain the modification"
+    Exit 1
+fi
+
+Exit 0
-- 
1.8.0