tests: Add a test case for path traversal in a dirtree

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

diff --git a/Makefile-tests.am b/Makefile-tests.am
index 350209dedb031f5be45264771e9095a33fcb23c7..284dc76f73716766f8a330eab158305c7c1a60dd 100644 (file)
--- a/Makefile-tests.am
+++ b/Makefile-tests.am
@@ -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)
 

diff --git a/cfg.mk b/cfg.mk
index 0eb05b897d46a9fa9eecfc3dd9bc5fbfb7dc3778..5947a141df56e29da70f1241d0edb5e8175fc6a4 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -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$$

diff --git a/tests/ostree-path-traverse.tar.gz b/tests/ostree-path-traverse.tar.gz
new file mode 100644 (file)
index 0000000..9dec3b7
Binary files /dev/null and b/tests/ostree-path-traverse.tar.gz differ

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index e6317fbf547244ded0db87194c63408e28019bae..463b41efb5e853438f5db68460be4aca9202414f 100644 (file)
--- a/tests/pull-test.sh
+++ b/tests/pull-test.sh
@@ -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
@@ -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

diff --git a/tests/test-corruption.sh b/tests/test-corruption.sh
index cb5e9c09eb08a5e330288699307ce94c5bef5f31..626929e7ab96d3245828b0a757cb201b70c0c3a8 100755 (executable)
--- a/tests/test-corruption.sh
+++ b/tests/test-corruption.sh
@@ -19,7 +19,7 @@
 
 set -euo pipefail
 
-echo "1..4"
+echo "1..5"
 
 . $(dirname $0)/libtest.sh
 
@@ -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"
+