git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/8] t3011: demonstrate directory traversal failures

From
Denton Liu <liu.denton@gmail.com>
Date
Dec 9, 2019, 21:06 UTC
Message-ID
<20191209210608.GA39599@generichostname>
In-Reply-To
<4b24ba9966d41558cc2382d81e6e76b4375b555c.1575924465.git.gitgitgadget@gmail.com>
Hi Elijah,
On Mon, Dec 09, 2019 at 08:47:38PM +0000, Elijah Newren via GitGitGadget wrote:
Show 43 quoted lines
> From: Elijah Newren <newren@gmail.com>
> 
> Add several tests demonstrating directory traversal failures of various
> sorts in dir.c (and one similar looking test that turns out to be a
> git_fnmatch bug).  A lot of these tests look like near duplicates of
> each other, but an optimization path in dir.c to pre-descend into a
> common prefix and the specialized treatment of trailing slashes in dir.c
> mean the tiny differences are sometimes important and potentially cause
> different codepaths to be explored.
> 
> Of the 7 failing tests, 2 are new to git-2.24.0 (tweaked by side effects
> of the en/clean-nested-with-ignored-topic); the other 5 also failed
> under git-2.23.0 and earlier.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  ...common-prefixes-and-directory-traversal.sh | 193 ++++++++++++++++++
>  1 file changed, 193 insertions(+)
>  create mode 100755 t/t3011-common-prefixes-and-directory-traversal.sh
> 
> diff --git a/t/t3011-common-prefixes-and-directory-traversal.sh b/t/t3011-common-prefixes-and-directory-traversal.sh
> new file mode 100755
> index 0000000000..773d6038d1
> --- /dev/null
> +++ b/t/t3011-common-prefixes-and-directory-traversal.sh
> @@ -0,0 +1,193 @@
> +#!/bin/sh
> +
> +test_description='directory traversal handling, especially with common prefixes'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	test_commit hello &&
> +
> +	>empty &&
> +	mkdir untracked_dir &&
> +	>untracked_dir/empty &&
> +	git init untracked_repo &&
> +	>untracked_repo/empty &&
> +
> +	echo ignored >.gitignore &&
> +	echo an_ignored_dir/ >>.gitignore &&
Could we just do
	cat <<-EOF >expect
	ignored
	an_ignored_dir/
	EOF

instead? Alternatively, we can use test_write_lines() but since we have here-doc usages below already, let's keep using here-docs here to be consistent. Same comment applies to anywhere in this patch where we have a >>.

Show 108 quoted lines
> +	mkdir an_ignored_dir &&
> +	mkdir an_untracked_dir &&
> +	>an_ignored_dir/ignored &&
> +	>an_ignored_dir/untracked &&
> +	>an_untracked_dir/ignored &&
> +	>an_untracked_dir/untracked
> +'
> +
> +test_expect_success 'git ls-files -o shows the right entries' '
> +	cat <<-EOF >expect &&
> +	.gitignore
> +	actual
> +	an_ignored_dir/ignored
> +	an_ignored_dir/untracked
> +	an_untracked_dir/ignored
> +	an_untracked_dir/untracked
> +	empty
> +	expect
> +	untracked_dir/empty
> +	untracked_repo/
> +	EOF
> +	git ls-files -o >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o --exclude-standard shows the right entries' '
> +	cat <<-EOF >expect &&
> +	.gitignore
> +	actual
> +	an_untracked_dir/untracked
> +	empty
> +	expect
> +	untracked_dir/empty
> +	untracked_repo/
> +	EOF
> +	git ls-files -o --exclude-standard >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o untracked_dir recurses' '
> +	echo untracked_dir/empty >expect &&
> +	git ls-files -o untracked_dir >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o untracked_dir/ recurses' '
> +	echo untracked_dir/empty >expect &&
> +	git ls-files -o untracked_dir/ >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o --directory untracked_dir does not recurse' '
> +	echo untracked_dir/ >expect &&
> +	git ls-files -o --directory untracked_dir >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_failure 'git ls-files -o --directory untracked_dir/ does not recurse' '
> +	echo untracked_dir/ >expect &&
> +	git ls-files -o --directory untracked_dir/ >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o untracked_repo does not recurse' '
> +	echo untracked_repo/ >expect &&
> +	git ls-files -o untracked_repo >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_failure 'git ls-files -o untracked_repo/ does not recurse' '
> +	echo untracked_repo/ >expect &&
> +	git ls-files -o untracked_repo/ >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_failure 'git ls-files -o untracked_dir untracked_repo recurses into untracked_dir only' '
> +	echo untracked_dir/empty >expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o untracked_dir untracked_repo >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o untracked_dir/ untracked_repo/ recurses into untracked_dir only' '
> +	echo untracked_dir/empty >expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o untracked_dir/ untracked_repo/ >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_failure 'git ls-files -o --directory untracked_dir untracked_repo does not recurse' '
> +	echo untracked_dir/ >expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o --directory untracked_dir untracked_repo >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o --directory untracked_dir/ untracked_repo/ does not recurse' '
> +	echo untracked_dir/ >expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o --directory untracked_dir/ untracked_repo/ >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o .git shows nothing' '
> +	>expect &&
> +	git ls-files -o .git >actual &&
> +	test_cmp expect actual
> +'
Could we use test_must_be_empty here instead?
Show 6 quoted lines
> +
> +test_expect_failure 'git ls-files -o .git/ shows nothing' '
> +	>expect &&
> +	git ls-files -o .git/ >actual &&
> +	test_cmp expect actual
> +'
test_must_be_empty?
Thanks,
Denton
Show 65 quoted lines
> +
> +test_expect_success FUNNYNAMES 'git ls-files -o untracked_* recurses appropriately' '
> +	mkdir "untracked_*" &&
> +	>"untracked_*/empty" &&
> +
> +	echo "untracked_*/empty" >expect &&
> +	echo untracked_dir/empty >>expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o "untracked_*" >actual &&
> +	test_cmp expect actual
> +'
> +
> +# It turns out fill_directory returns the right paths, but ls-files' post-call
> +# filtering in show_dir_entry() via calling dir_path_match() which ends up
> +# in git_fnmatch() has logic for PATHSPEC_ONESTAR that assumes the pathspec
> +# must match the full path; it doesn't check it for matching a leading
> +# directory.
> +test_expect_failure FUNNYNAMES 'git ls-files -o untracked_*/ recurses appropriately' '
> +	echo "untracked_*/empty" >expect &&
> +	echo untracked_dir/empty >>expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o "untracked_*/" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success FUNNYNAMES 'git ls-files -o --directory untracked_* does not recurse' '
> +	echo "untracked_*/" >expect &&
> +	echo untracked_dir/ >>expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o --directory "untracked_*" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success FUNNYNAMES 'git ls-files -o --directory untracked_*/ does not recurse' '
> +	echo "untracked_*/" >expect &&
> +	echo untracked_dir/ >>expect &&
> +	echo untracked_repo/ >>expect &&
> +	git ls-files -o --directory "untracked_*/" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files -o consistent between one or two dirs' '
> +	git ls-files -o --exclude-standard an_ignored_dir/ an_untracked_dir/ >tmp &&
> +	! grep ^an_ignored_dir/ tmp >expect &&
> +	git ls-files -o --exclude-standard an_ignored_dir/ >actual &&
> +	test_cmp expect actual
> +'
> +
> +# ls-files doesn't have a way to request showing both untracked and ignored
> +# files at the same time, so use `git status --ignored`
> +test_expect_failure 'git status --ignored shows same files under dir with or without pathspec' '
> +	cat <<-EOF >expect &&
> +	?? an_untracked_dir/
> +	!! an_untracked_dir/ignored
> +	EOF
> +	git status --porcelain --ignored >output &&
> +	grep an_untracked_dir output >expect &&
> +	git status --porcelain --ignored an_untracked_dir/ >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_done
> -- 
> gitgitgadget
> 
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 13 of 69 in “Directory traversal bugs”
  1. 0/8 Directory traversal bugsElijah Newren via GitGitGadget, Dec 9, 2019
  2. 2/8 Revert "dir.c: make 'git-status --ignored' work within leading directories"Elijah Newren via GitGitGadget, Dec 9, 2019
  3. Denton LiuDec 9, 2019
  4. Elijah NewrenDec 9, 2019
  5. Eric SunshineDec 9, 2019
  6. 3/8 dir: remove stray quote character in commentElijah Newren via GitGitGadget, Dec 9, 2019
  7. 5/8 dir: break part of read_directory_recursive() out for reuseElijah Newren via GitGitGadget, Dec 9, 2019
  8. 7/8 dir: synchronize treat_leading_path() and read_directory_recursive()Elijah Newren via GitGitGadget, Dec 9, 2019
  9. 8/8 dir: consolidate similar code in treat_directory()Elijah Newren via GitGitGadget, Dec 9, 2019
  10. 6/8 dir: fix checks on common prefix directoryElijah Newren via GitGitGadget, Dec 9, 2019
  11. 4/8 dir: exit before wildcard fall-through if there is no wildcardElijah Newren via GitGitGadget, Dec 9, 2019
  12. 1/8 t3011: demonstrate directory traversal failuresElijah Newren via GitGitGadget, Dec 9, 2019
  13. Denton LiuDec 9, 2019
  14. 0/8 Directory traversal bugsElijah Newren via GitGitGadget, Dec 10, 2019
  15. 2/8 Revert "dir.c: make 'git-status --ignored' work within leading directories"Elijah Newren via GitGitGadget, Dec 10, 2019
  16. 1/8 t3011: demonstrate directory traversal failuresElijah Newren via GitGitGadget, Dec 10, 2019
  17. 6/8 dir: fix checks on common prefix directoryElijah Newren via GitGitGadget, Dec 10, 2019
  18. Johannes SchindelinDec 15, 2019
  19. Elijah NewrenDec 16, 2019
  20. Elijah NewrenDec 16, 2019
  21. Junio C HamanoDec 16, 2019
  22. Elijah NewrenDec 16, 2019
  23. Junio C HamanoDec 16, 2019
  24. Elijah NewrenDec 16, 2019
  25. Johannes SchindelinDec 17, 2019
  26. Junio C HamanoDec 17, 2019
  27. Johannes SchindelinDec 17, 2019
  28. Junio C HamanoDec 17, 2019
  29. Johannes SchindelinDec 17, 2019
  30. Elijah NewrenDec 17, 2019
  31. Johannes SchindelinDec 17, 2019
  32. Elijah NewrenDec 17, 2019
  33. 3/8 dir: remove stray quote character in commentElijah Newren via GitGitGadget, Dec 10, 2019
  34. 8/8 dir: consolidate similar code in treat_directory()Elijah Newren via GitGitGadget, Dec 10, 2019
  35. 5/8 dir: break part of read_directory_recursive() out for reuseElijah Newren via GitGitGadget, Dec 10, 2019
  36. 4/8 dir: exit before wildcard fall-through if there is no wildcardElijah Newren via GitGitGadget, Dec 10, 2019
  37. 7/8 dir: synchronize treat_leading_path() and read_directory_recursive()Elijah Newren via GitGitGadget, Dec 10, 2019
  38. 0/3 Directory traversal bugsElijah Newren via GitGitGadget, Dec 17, 2019
  39. 2/3 dir: remove stray quote character in commentElijah Newren via GitGitGadget, Dec 17, 2019
  40. 3/3 dir: exit before wildcard fall-through if there is no wildcardElijah Newren via GitGitGadget, Dec 17, 2019
  41. 1/3 t3011: demonstrate directory traversal failuresElijah Newren via GitGitGadget, Dec 17, 2019
  42. Johannes SchindelinDec 17, 2019
  43. Junio C HamanoDec 17, 2019
  44. Johannes SchindelinDec 21, 2019
  45. 0/8 Directory traversal bugsElijah Newren via GitGitGadget, Dec 18, 2019
  46. 2/8 Revert "dir.c: make 'git-status --ignored' work within leading directories"Elijah Newren via GitGitGadget, Dec 18, 2019
  47. 1/8 t3011: demonstrate directory traversal failuresElijah Newren via GitGitGadget, Dec 18, 2019
  48. 3/8 dir: remove stray quote character in commentElijah Newren via GitGitGadget, Dec 18, 2019
  49. 4/8 dir: exit before wildcard fall-through if there is no wildcardElijah Newren via GitGitGadget, Dec 18, 2019
  50. 5/8 dir: break part of read_directory_recursive() out for reuseElijah Newren via GitGitGadget, Dec 18, 2019
  51. 6/8 dir: fix checks on common prefix directoryElijah Newren via GitGitGadget, Dec 18, 2019
  52. Junio C HamanoDec 18, 2019
  53. Elijah NewrenDec 19, 2019
  54. Jeff KingDec 19, 2019
  55. Elijah NewrenDec 20, 2019
  56. Jeff KingDec 20, 2019
  57. Junio C HamanoDec 20, 2019
  58. Jeff KingDec 20, 2019
  59. 8/8 dir: consolidate similar code in treat_directory()Elijah Newren via GitGitGadget, Dec 18, 2019
  60. 7/8 dir: synchronize treat_leading_path() and read_directory_recursive()Elijah Newren via GitGitGadget, Dec 18, 2019
  61. 0/8 Directory traversal bugsElijah Newren via GitGitGadget, Dec 19, 2019
  62. 1/8 t3011: demonstrate directory traversal failuresElijah Newren via GitGitGadget, Dec 19, 2019
  63. 5/8 dir: break part of read_directory_recursive() out for reuseElijah Newren via GitGitGadget, Dec 19, 2019
  64. 4/8 dir: exit before wildcard fall-through if there is no wildcardElijah Newren via GitGitGadget, Dec 19, 2019
  65. 3/8 dir: remove stray quote character in commentElijah Newren via GitGitGadget, Dec 19, 2019
  66. 2/8 Revert "dir.c: make 'git-status --ignored' work within leading directories"Elijah Newren via GitGitGadget, Dec 19, 2019
  67. 6/8 dir: fix checks on common prefix directoryElijah Newren via GitGitGadget, Dec 19, 2019
  68. 8/8 dir: consolidate similar code in treat_directory()Elijah Newren via GitGitGadget, Dec 19, 2019
  69. 7/8 dir: synchronize treat_leading_path() and read_directory_recursive()Elijah Newren via GitGitGadget, Dec 19, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.