From 2b78df25f469f01f96616ac3bcb5bc17bd68ab2e Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Fri, 12 Jan 2018 09:01:52 -0500
Subject: [PATCH] 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
---
 Makefile-tests.am                 |   1 +
 cfg.mk                            |   2 +-
 tests/ostree-path-traverse.tar.gz | Bin 0 -> 1265 bytes
 tests/pull-test.sh                |  17 ++++++++++++++++-
 tests/test-corruption.sh          |  12 +++++++++++-
 5 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 tests/ostree-path-traverse.tar.gz

diff --git a/Makefile-tests.am b/Makefile-tests.am
index 350209de..284dc76f 100644
--- 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 0eb05b89..5947a141 100644
--- 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
index 0000000000000000000000000000000000000000..9dec3b73c76a149d34db24015d4da5918dd96558
GIT binary patch
literal 1265
zcmV<N1P=QjiwFQT?pIm>1MQqoXk0}c$F~*ylYoDEXwg#0L5OLyoB2QQ9a^Ilu^PN6
z6e>k$=FLmCcK3z%HX#Hr9t!m)coE5=XuXJ7w6vg}Jc*voMW`SeFSY>@JXAs7+axw=
zvU$7N-FZp<eGZvNW?^Rb`+hUu-@G>)C5_nH^0aD9l^d~|wQ*v{O1%Z8NXfGjOODo;
zYm_i0gi9e2CnbcbkhC<!d)$vLnMoQd9vVtZ8*l%)?d$g_(N_A$b~@_eP0-Yzx23<J
zEd3m2f)UW)1+DcrQ9Z0q_M(kUV8K{x{h9br|MX9)zd)3ghEOjW&*Oit|Eng7?b(4$
z@z*l(t5N+p9;iiuJ+5LiRh_lvc`{IRFM)p1I{q8A=^m{CozMTA!Tj%r?e#axOm%Mu
zJMe#N^FPNJ^mjpP{q0=C*0c2>>GlTbB>q?QmxzJ?cS39Z)9<Cxn^drK{nIN3`n#aD
z{;>_aj{FsNpnsYGfc`GX(SORS!0G(I;{SvY9Q1cVTl#0UYnA1OO&XoGk(d8tgg5F>
z2+8z^{J;MopCiNwHx^r?1@U#d`1(eL*po(~&?Az0+$Y>>FASLRJtYFAeXBxi6h^Q$
zu(!#H|5`PUqqxfzU<dx6rvHNZ-vK%L$F>%w6TWl(iGcjy39a>y^f_x9y+{Fe;Qt6b
z|8qmW{*3j+|F^mSx2iuUkpDZOGFBNIpNQ&{6V>_~eQIN!|8Hu2SD9`1Z@3o_#?AbH
zqEG$Vz?%MX6m6T<<kjy{+KPXnkPR``(ikjcNOdZY<sLWErf4B~s1(B_;MPbQrdfpN
zS&g(XoOxa)sKzzhP(=?F*a8>&=jHzhvpfDz&@}&-3<=2p-SW(>qI&?iCqMorEyVu`
z@P97Y9sZAO_J0`sp9|WG|G@e*P+T&?6+;qfj<n?gSB44dQx*h-rN)2~sR{D5F9=08
z2#w&1U{ndK=k0va0|mCkh5otzPl&txpOBb<|8v9>>#h&nxqRz#vv~a2^P}HBHS)sf
zi{+y)EtXG}D?jab+drU><j240`X!(p_&<Rp_&*oy4*v)He?`$&{8NUsZ^BShseI2j
ziZf<~*3v_Y80q;42Q;mvl6q2j9Aib1z*2=Ot;zQPvPOy?DzF7E^w0Hw>)!ua-Ty;r
z0sapF`b>WOt3v#rfb&020RIR6FE{>u#e;NB#;W;bPAOt7#ZoIyHKvx@P@|Ce0{JFj
zo-ZucDzw%zqk=$l@PE4)7y9S<KfG@LZ{_`ODG8kac0#2xrn3F{Qu5=wKb(H{%=q-R
z!KKAdp7Ys_Cl4mWpFjA8Mz{X9A3XZd(PT!akzA^?!G(XW&cC<t(aB%Gnj09Md*{g0
zSEV<5;js_zKXm2Nk(1{h{{8TqUr+vV{l29O-z+a*pSgYcz~as1&ykD24W57H;_w5f
zjuky>V2%9vM}8sCf8qR(6LyFHqnrI70srTMw&K6LBSIAu)oRsy@yojhaG`&$|0A3H
z9~1EW*EwfS-(33i{j<aSE)9==T>1IXmoI;I;>2H%jJ&?^{oD1!SC)VI?#8t|cZc@x
zD|#3}2egj=nR+^JPscV%Y*1dE49YwDL71lh>*V|wA@KVj00000000000000000000
b000000000000000unqqLd};Rh0C)fZpA?^(

literal 0
HcmV?d00001

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index e6317fbf..463b41ef 100644
--- 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 cb5e9c09..626929e7 100755
--- 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"
+