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

Re: [RFC] submodule: munge paths to submodule git directories

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Jan 15, 2019, 01:25 UTC
Message-ID
<20190115012507.GK162110@google.com>
In-Reply-To
<20180807230637.247200-1-bmwill@google.com>
Hi,
In August, 2018, Brandon Williams wrote:
Show 8 quoted lines
> Commit 0383bbb901 (submodule-config: verify submodule names as paths,
> 2018-04-30) introduced some checks to ensure that submodule names don't
> include directory traversal components (e.g. "../").
>
> This addresses the vulnerability identified in 0383bbb901 but the root
> cause is that we use submodule names to construct paths to the
> submodule's git directory.  What we really should do is munge the
> submodule name before using it to construct a path.

Thanks again for this. I liked the proposal enough to run Git with patches implementing it for a while. That said, there were some unaddressed comments in the review.

I've put a summary in https://crbug.com/git/28 to make this easier to pick up where we left off. Summary from there of the upstream review:

1. Using urlencoding to escape the slashes is fine, but what if we
   want to escape some other character (for example to handle
   case-insensitive filesystems)?
   Proposal: Store the escaping mapping in config[1] so it can be
   modified it in the future:
	[submodule "plugin/hooks"]
		gitdirname = plugins%2fhooks
2. The urlencoded name could conflict with a submodule that has % in
   its name in an existing clone created by an older version of Git.
   Proposal: Put submodules in a new .git/submodules/ directory
   instead of .git/modules/.
3. These gitdirname settings can clutter up .git/config.
   Proposal: For the "easy" cases (e.g. submodule name consisting of
   [a-z]*), allow omitting the gitdirname setting.

Is that a fair summary? Are there concerns from the review that I forgot, or would a new version of the series that addresses those three problems put us in good shape?

Thanks, Jonathan

[1] https://public-inbox.org/git/20180816181940.46114-1-bmwill@google.com/
Previous: Aaron SchrabNext: Jeff King
Message 38 of 40 in “[RFC] submodule: munge paths to submodule git directories”
  1. Brandon WilliamsAug 7, 2018
  2. Jonathan NiederAug 7, 2018
  3. Junio C HamanoAug 8, 2018
  4. 0/2 munge submodule namesBrandon Williams, Aug 8, 2018
  5. 1/2 submodule: create helper to build paths to submodule gitdirsBrandon Williams, Aug 8, 2018
  6. Stefan BellerAug 8, 2018
  7. Brandon WilliamsAug 9, 2018
  8. Junio C HamanoAug 10, 2018
  9. Brandon WilliamsAug 10, 2018
  10. 2/2 submodule: munge paths to submodule git directoriesBrandon Williams, Aug 8, 2018
  11. Jeff KingAug 9, 2018
  12. Brandon WilliamsAug 14, 2018
  13. Jonathan NiederAug 14, 2018
  14. Stefan BellerAug 14, 2018
  15. Jonathan NiederAug 14, 2018
  16. Stefan BellerAug 14, 2018
  17. Jonathan NiederAug 16, 2018
  18. Stefan BellerAug 16, 2018
  19. Jonathan NiederAug 16, 2018
  20. Brandon WilliamsAug 16, 2018
  21. submodule: add config for where gitdirs are locatedBrandon Williams, Aug 16, 2018
  22. Junio C HamanoAug 20, 2018
  23. Junio C HamanoAug 16, 2018
  24. Jeff KingAug 14, 2018
  25. Stefan BellerAug 28, 2018
  26. Jeff KingAug 29, 2018
  27. Stefan BellerAug 29, 2018
  28. Jeff KingAug 29, 2018
  29. Stefan BellerAug 29, 2018
  30. Jonathan NiederAug 29, 2018
  31. Stefan BellerAug 29, 2018
  32. Jeff KingAug 29, 2018
  33. Jonathan NiederAug 29, 2018
  34. Stefan BellerAug 29, 2018
  35. Brandon WilliamsAug 29, 2018
  36. Jeff KingAug 29, 2018
  37. Aaron SchrabAug 16, 2018
  38. Jonathan NiederJan 15, 2019
  39. Jeff KingJan 17, 2019
  40. Stefan BellerJan 17, 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.