Overwriting Symbolic Links with Files in Copy Provisioner. When a layer overwrites a symbolic link with a regular file, the link must be removed first, otherwise 'cp' would follow the link and overwrite the target instead of the link itself.
Review: https://reviews.apache.org/r/58463/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3c8deedc Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3c8deedc Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3c8deedc Branch: refs/heads/master Commit: 3c8deedc9a1bce617965c3442713ebdc6691d1ae Parents: bc12a58 Author: Chun-Hung Hsiao <[email protected]> Authored: Tue Apr 18 14:18:45 2017 +0800 Committer: Jie Yu <[email protected]> Committed: Tue Apr 18 14:18:45 2017 +0800 ---------------------------------------------------------------------- .../mesos/provisioner/backends/copy.cpp | 28 +++++++++++++------- 1 file changed, 19 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/3c8deedc/src/slave/containerizer/mesos/provisioner/backends/copy.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp index 68178cb..69faa03 100644 --- a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp +++ b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp @@ -190,15 +190,25 @@ Future<Nothing> CopyBackendProcess::_provision( } } - // Handle overwriting between directories and non-directories. - // Note: If a symbolic link is overwritten by a directory, the - // symbolic link must be removed before the directory is traversed - // so the following case won't cause a security issue: - // ROOTFS: /bad@ -> /usr - // LAYER: /bad/bin/.wh.wh.opq - bool ftsIsDir = node->fts_info == FTS_D || node->fts_info == FTS_DC; - if (os::exists(rootfsPath) && os::stat::isdir(rootfsPath) != ftsIsDir) { - removePath = rootfsPath; + if (os::exists(rootfsPath)) { + bool ftsIsDir = node->fts_info == FTS_D || node->fts_info == FTS_DC; + if (os::stat::isdir(rootfsPath) != ftsIsDir) { + // Handle overwriting between a directory and a non-directory. + // Note: If a symlink is overwritten by a directory, the symlink + // must be removed before the directory is traversed so the + // following case won't cause a security issue: + // ROOTFS: /bad@ -> /usr + // LAYER: /bad/bin/.wh.wh.opq + removePath = rootfsPath; + } else if (os::stat::islink(rootfsPath)) { + // Handle overwriting a symlink with a regular file. + // Note: The symlink must be removed, or 'cp' would follow the + // link and overwrite the target instead of the link itself, + // which would cause a security issue in the following case: + // ROOTFS: /bad@ -> /usr/bin/python + // LAYER: /bad is a malicious executable + removePath = rootfsPath; + } } // The file/directory referred to by removePath may be empty or have
