From f3ae36ff4360c58158963ca2c20862ae94ac0775 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Fri, 12 Jan 2018 10:40:36 -0500
Subject: [PATCH] lib/checkout: Validate pathnames during checkout

While we do protect against path traversal during pull, let's also validate
during checkout; it's a cheap operation and provides good last-mile protection.

Closes: #1412
Approved by: jlebon
---
 src/libostree/ostree-repo-checkout.c | 13 +++++++++++++
 tests/test-corruption.sh             | 11 ++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index 962b503d12..6334ae508b 100644
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -535,6 +535,10 @@ checkout_one_file_at (OstreeRepo                        *repo,
                       GCancellable                      *cancellable,
                       GError                           **error)
 {
+  /* Validate this up front to prevent path traversal attacks */
+  if (!ot_util_filename_validate (destination_name, error))
+    return FALSE;
+
   gboolean need_copy = TRUE;
   gboolean is_bare_user_symlink = FALSE;
   char loose_path_buf[_OSTREE_LOOSE_PATH_MAX];
@@ -897,6 +901,15 @@ checkout_tree_at_recurse (OstreeRepo                        *self,
     while (g_variant_iter_loop (&viter, "(&s@ay@ay)", &dname,
                                 &subdirtree_csum_v, &subdirmeta_csum_v))
       {
+        /* Validate this up front to prevent path traversal attacks. Note that
+         * we don't validate at the top of this function like we do for
+         * checkout_one_file_at() becuase I believe in some cases this function
+         * can be called *initially* with user-specified paths for the root
+         * directory.
+         */
+        if (!ot_util_filename_validate (dname, error))
+          return FALSE;
+
         const size_t origlen = selabel_path_buf ? selabel_path_buf->len : 0;
         if (selabel_path_buf)
           {
diff --git a/tests/test-corruption.sh b/tests/test-corruption.sh
index 626929e7ab..74ce8e835e 100755
--- a/tests/test-corruption.sh
+++ b/tests/test-corruption.sh
@@ -19,7 +19,7 @@
 
 set -euo pipefail
 
-echo "1..5"
+echo "1..6"
 
 . $(dirname $0)/libtest.sh
 
@@ -79,6 +79,11 @@ if ${CMD_PREFIX} ostree --repo=ostree-path-traverse/repo fsck -q 2>err.txt; then
     fatal "fsck unexpectedly succeeded"
 fi
 assert_file_has_content_literal err.txt '.dirtree: Invalid / in filename ../afile'
+echo "ok path traverse fsck"
 
-echo "ok path traverse"
-
+cd ${test_tmpdir}
+if ${CMD_PREFIX} ostree --repo=ostree-path-traverse/repo checkout pathtraverse-test pathtraverse-test 2>err.txt; then
+    fatal "checkout with path traversal unexpectedly succeeded"
+fi
+assert_file_has_content_literal err.txt 'Invalid / in filename ../afile'
+echo "ok path traverse checkout"