Skip to content

Commit

Permalink
lib/checkout: Validate pathnames during checkout
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Jan 12, 2018
1 parent 2b78df2 commit f3ae36f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
13 changes: 13 additions & 0 deletions src/libostree/ostree-repo-checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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)
{
Expand Down
11 changes: 8 additions & 3 deletions tests/test-corruption.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

set -euo pipefail

echo "1..5"
echo "1..6"

. $(dirname $0)/libtest.sh

Expand Down Expand Up @@ -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"

0 comments on commit f3ae36f

Please sign in to comment.