Skip to content

Commit

Permalink
tests: Add a test case for path traversal in a dirtree
Browse files Browse the repository at this point in the history
I was reading about a recent security issue with both EMC and VMWare:
https://arstechnica.com/information-technology/2018/01/emc-vmware-security-bugs-throw-gasoline-on-cloud-security-fire/

It's a classic path traversal problem, and that made me think more about our
handling of this in libostree.  Fortunately of course, not being new to
this rodeo, long ago I *did* consider path traversal.  Inside the pull
code, we call `ot_util_filename_validate()`.  Also, `fsck` does this too.

I have further followups here, but let's add some test cases for this. I crafted
a repository with a `../` in a dirtree object by patching libostree to inject
it, and that's included as a tarball.

This patch covers the two cases where we do already have checks; pulling
via HTTP, and in `fsck`.

Closes: #1412
Approved by: jlebon
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Jan 12, 2018
1 parent 854a823 commit 2b78df2
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 3 deletions.
1 change: 1 addition & 0 deletions Makefile-tests.am
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ dist_installed_test_data = tests/archive-test.sh \
tests/pre-endian-deltas-repo-little.tar.xz \
tests/fah-deltadata-old.tar.xz \
tests/fah-deltadata-new.tar.xz \
tests/ostree-path-traverse.tar.gz \
tests/libtest-core.sh \
$(NULL)

Expand Down
2 changes: 1 addition & 1 deletion cfg.mk
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ sc_glnx_no_fd_close:
show-vc-list-except:
@$(VC_LIST_EXCEPT)

VC_LIST_ALWAYS_EXCLUDE_REGEX = ^ABOUT-NLS|cfg.mk|maint.mk|*.gpg|*.sig|.xz$$
VC_LIST_ALWAYS_EXCLUDE_REGEX = ^ABOUT-NLS|cfg.mk|maint.mk|*.gpg|*.sig|.xz|.gz$$
Binary file added tests/ostree-path-traverse.tar.gz
Binary file not shown.
17 changes: 16 additions & 1 deletion tests/pull-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function verify_initial_contents() {
assert_file_has_content baz/cow '^moo$'
}

echo "1..33"
echo "1..34"

# Try both syntaxes
repo_init --no-gpg-verify
Expand Down Expand Up @@ -217,6 +217,21 @@ else
echo "ok corruption (skipped)"
fi


cd ${test_tmpdir}/ostree-srv
tar xf ${test_srcdir}/ostree-path-traverse.tar.gz
cd ${test_tmpdir}
rm corruptrepo -rf
ostree_repo_init corruptrepo --mode=archive
${CMD_PREFIX} ostree --repo=corruptrepo remote add --set=gpg-verify=false pathtraverse $(cat httpd-address)/ostree/ostree-path-traverse/repo
if ${CMD_PREFIX} ostree --repo=corruptrepo pull pathtraverse pathtraverse-test 2>err.txt; then
fatal "Pulled a repo with path traversal in dirtree"
fi
assert_file_has_content_literal err.txt 'Invalid / in filename ../afile'
rm corruptrepo -rf
echo "ok path traversal checked on pull"


cd ${test_tmpdir}
rm mirrorrepo/refs/remotes/* -rf
${CMD_PREFIX} ostree --repo=mirrorrepo prune --refs-only
Expand Down
12 changes: 11 additions & 1 deletion 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..4"
echo "1..5"

. $(dirname $0)/libtest.sh

Expand Down Expand Up @@ -72,3 +72,13 @@ fi
assert_file_has_content_literal err.txt "Loading commit for ref test2: No such metadata object"

echo "ok missing commit"

cd ${test_tmpdir}
tar xf ${test_srcdir}/ostree-path-traverse.tar.gz
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"

0 comments on commit 2b78df2

Please sign in to comment.