{"thread":{"id":"55816","subject":"proposal for additional search path in config","startedAt":"2021-06-01T14:13:34Z","lastAt":"2021-06-03T02:46:33Z","messageCount":4,"participants":["Jordi Vilar","Johannes Schindelin","Taylor Blau"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"426126","messageId":"CAE-zgtYTutLZWO63QtqZJztMPqi9eHYQfFu6sda8YDf_bVeE3Q@mail.gmail.com","threadId":"55816","inReplyTo":"20210601113554.52585C06174A@lindbergh.monkeyblade.net","subject":"proposal for additional search path in config","fromName":"Jordi Vilar","fromEmail":"git.kernel.org@jordi.vilar.cat","sentAt":"2021-06-01T14:13:18Z","receivedAt":"2021-06-01T14:13:34Z","isPatch":false,"sender":{"key":"git.kernel.org@jordi.vilar.cat","avatar":null},"body":"Hi folks,\n\nI would like to gather feedback about a feature I'm considering about.\nBasically, I would like to propose a configuration setting with a path\n(possibly relative to the working tree) to look for executables. In\nthat way, a local clone could setup additional tools/scripts private\nfor its work tree, maybe also under revision control.\n\nFor instance, we could have a setting like:\n  git config core.extensions_path \"./scripts\"\nso that whenever git has to resolve an external command also includes\nthat path in the search, resolving relative paths to the work tree, in\nthis case, the \"/scripts\" subdir.\n\nA simple use case could be a custom tool integrated with git, but\nlocal to the repository and thus versioned.\n\nFor instance, let's assume that we have some metadata required by a\ncustom build tool that is stored in commit comments or tags, so we\nhave a custom script git-project-metadata to handle it:\n  git project-metadata query ...\n\nMaybe the format for the metadata evolves, so the script must evolve\nalso. Having it integrated in this way would allow that each checkout\nsees the correct script version, and that script is always invoked as\na regular git command, without having to tweak the environment\neverytime.\n\nIt is not clear to me whether it's a good idea for this additional\npath to have priority over the default search path, as in one hand it\ncould allow to override the default tools, but in the other hand this\nmay be a hole for bad things... For my, the conservative approach of\nhaving this additional path as a fallback is ok, so that it only adds\nmore commands, but doesn't override commands.\n\nAlso, this setting could be set either globally or locally, and in\nthis case, in the conservative approach, the overloading rule is the\ndifferent to the normal one: the system search path in the environment\npath variable has precedence over the global setting and this over the\nlocal setting, but all apply.\n\nDoes it sound reasonable? Is there interest in such feature?\n\nThanks,\n\nJordi\n"},{"id":"426218","messageId":"nycvar.QRO.7.76.6.2106021259480.55@tvgsbejvaqbjf.bet","threadId":"55816","inReplyTo":"CAE-zgtYTutLZWO63QtqZJztMPqi9eHYQfFu6sda8YDf_bVeE3Q@mail.gmail.com","subject":"Re: proposal for additional search path in config","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-06-02T11:08:39Z","receivedAt":"2021-06-02T11:08:44Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jordi,\n\nOn Tue, 1 Jun 2021, Jordi Vilar wrote:\n\n> I would like to gather feedback about a feature I'm considering about.\n> Basically, I would like to propose a configuration setting with a path\n> (possibly relative to the working tree) to look for executables. In\n> that way, a local clone could setup additional tools/scripts private\n> for its work tree, maybe also under revision control.\n\nThat sounds like a Remote Code Execution vulnerability by design in the\nmaking. But let's read on.\n\n> For instance, we could have a setting like:\n>   git config core.extensions_path \"./scripts\"\n> so that whenever git has to resolve an external command also includes\n> that path in the search, resolving relative paths to the work tree, in\n> this case, the \"/scripts\" subdir.\n>\n> A simple use case could be a custom tool integrated with git, but\n> local to the repository and thus versioned.\n>\n> For instance, let's assume that we have some metadata required by a\n> custom build tool that is stored in commit comments or tags, so we\n> have a custom script git-project-metadata to handle it:\n>   git project-metadata query ...\n>\n> Maybe the format for the metadata evolves, so the script must evolve\n> also. Having it integrated in this way would allow that each checkout\n> sees the correct script version, and that script is always invoked as\n> a regular git command, without having to tweak the environment\n> everytime.\n>\n> It is not clear to me whether it's a good idea for this additional\n> path to have priority over the default search path, as in one hand it\n> could allow to override the default tools, but in the other hand this\n> may be a hole for bad things... For my, the conservative approach of\n> having this additional path as a fallback is ok, so that it only adds\n> more commands, but doesn't override commands.\n>\n> Also, this setting could be set either globally or locally, and in\n> this case, in the conservative approach, the overloading rule is the\n> different to the normal one: the system search path in the environment\n> path variable has precedence over the global setting and this over the\n> local setting, but all apply.\n>\n> Does it sound reasonable? Is there interest in such feature?\n\nI am really wary about the security implications of this. Most obviously\nwith the idea of allowing to _override_ commands. Take for example\n`git-lfs`: the assumption is that it is safe for Git to call `git-lfs`\nafter the initial check-out phase, but with this feature, it would be\npossible for Git to clone a malicious repository and _immediately_\nexecuting code it just cloned, _without_ giving the user a chance to even\ninspect the code.\n\nBut even if you _append_ that path to the `PATH` variable, unintended\nvulnerabilities could be opened. Take for example `git difftool`, which\nlooks for executables in the path until it finds one that is installed. An\nattacker could guess which executables your setup is missing, and squat on\nthose, overriding your `git difftool` to execute code you did not intend\nto be executed.\n\nSo putting a security lens before my eyes, I would be quite worried about\nsuch a feature.\n\nNote that the _use case_ you present is a common one, and what I see\nprojects do is to integrate the configuration into their build process\n(carefully vetting any code changes to the build process is _always_ a\ngood idea, hence this is a lot safer than having Git configure what is run\nautomatically). And of course, this is already possible right now, it just\ndelays configuration of those included tools to the time of the first\nbuild, as opposed to the clone time (as suggested above).\n\nCiao,\nJohannes\n"},{"id":"426243","messageId":"CAE-zgtZroyEwG1k9y-XXAx2NKPF=Lav4YG+f7mF227FEeuxDVw@mail.gmail.com","threadId":"55816","inReplyTo":"nycvar.QRO.7.76.6.2106021259480.55@tvgsbejvaqbjf.bet","subject":"Re: proposal for additional search path in config","fromName":"Jordi Vilar","fromEmail":"git.kernel.org@jordi.vilar.cat","sentAt":"2021-06-02T17:36:31Z","receivedAt":"2021-06-02T17:38:01Z","isPatch":false,"sender":{"key":"git.kernel.org@jordi.vilar.cat","avatar":null},"body":"Hi Johannes,\n\nThank you for taking the time to read the proposal and reply with your comments!\n\n> That sounds like a Remote Code Execution vulnerability by design in the\n> making. But let's read on.\n\nI agree with you that a too simple implementation of this would pose\nan unacceptable risk. But I think the details are important in order\nto assess the actual issues. Let me develop those details as I reply\nto your comments.\n\n> I am really wary about the security implications of this. Most obviously\n> with the idea of allowing to _override_ commands. Take for example\n> `git-lfs`: the assumption is that it is safe for Git to call `git-lfs`\n> after the initial check-out phase, but with this feature, it would be\n> possible for Git to clone a malicious repository and _immediately_\n> executing code it just cloned, _without_ giving the user a chance to even\n> inspect the code.\n\nYou are again right. That's why I was suggesting the conservative\napproach of not prepending to the default search path, but appending\nto it, so there is no chance of overrinding existing tools. Also,\nconfig is not versioned, so, right after cloning you wouldn't have\nthis option enabled, so you are always safe after cloning. Then you\nshould have to manually configure it, of course, only for trusted\nrepositories. But if you configure this search path in the global\nsettings, you should avoid setting a relative path (or maybe git could\nban it). But in any case, only the user can set this, it is not\nautomatic upon cloning at all.\n\n> But even if you _append_ that path to the `PATH` variable, unintended\n> vulnerabilities could be opened. Take for example `git difftool`, which\n> looks for executables in the path until it finds one that is installed. An\n> attacker could guess which executables your setup is missing, and squat on\n> those, overriding your `git difftool` to execute code you did not intend\n> to be executed.\n\nWell, the apparent effect would be like appending to the PATH\nvariable, but I'm not suggesting to implement it in that way. I didn't\nsay it explicitly, but what I had in mind is that git searches for\nextensions also in this additional path, but without modifying the\nenvironment variable, so it is not inherited by any executable or any\nother built-in git functionality.\n\n> So putting a security lens before my eyes, I would be quite worried about\n> such a feature.\n\nAnd I appreciate your critical thinking. I just want to share an idea\nI think could be useful, not to open a hole that we could later regret\nto have open, so all precautions are good.\n\n> Note that the _use case_ you present is a common one, and what I see\n> projects do is to integrate the configuration into their build process\n> (carefully vetting any code changes to the build process is _always_ a\n> good idea, hence this is a lot safer than having Git configure what is run\n> automatically). And of course, this is already possible right now, it just\n> delays configuration of those included tools to the time of the first\n> build, as opposed to the clone time (as suggested above).\n\nFor sure, decoupling sources/resources from tools is optimal. But not\nalways possible. And having to setup the environment for each worktree\nmakes working with multiple worktrees at a time from the same shell\nsession a pain and error prone.\n\nRegards,\n\nJordi\n"},{"id":"426287","messageId":"YLhCPMMhD9F3qH/l@nand.local","threadId":"55816","inReplyTo":"CAE-zgtZroyEwG1k9y-XXAx2NKPF=Lav4YG+f7mF227FEeuxDVw@mail.gmail.com","subject":"Re: proposal for additional search path in config","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-06-03T02:45:16Z","receivedAt":"2021-06-03T02:46:33Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jun 02, 2021 at 07:36:31PM +0200, Jordi Vilar wrote:\n> > I am really wary about the security implications of this. Most obviously\n> > with the idea of allowing to _override_ commands. Take for example\n> > `git-lfs`: the assumption is that it is safe for Git to call `git-lfs`\n> > after the initial check-out phase, but with this feature, it would be\n> > possible for Git to clone a malicious repository and _immediately_\n> > executing code it just cloned, _without_ giving the user a chance to even\n> > inspect the code.\n>\n> You are again right. That's why I was suggesting the conservative\n> approach of not prepending to the default search path, but appending\n> to it, so there is no chance of overrinding existing tools.\n\nTo me, this does not appear to be a conservative approach as you\nsuggest.\n\nThe only difference between exporting PATH=$SEARCH_PATH:$PATH and\nPATH=$PATH:$SEARCH_PATH is that the former allows overwriting the\nresults of looking up a binary in the path, but the latter lets you\nresolve locations that $PATH alone would not find.\n\nSuppose you maliciously included a git-lfs binary with your repository.\nIf you include that binary in your PATH ahead of the existing system\nPATH, then you'll replace system git-lfs with your malicious one, which\nI think you and I both agree is bad. But if you instead append the\nmalicious binary onto the right-hand side of your PATH, then you can't\noverwrite a git-lfs already on the path, but you *can* trick a system\nwhich doesn't have a version of git-lfs installed into thinking that one\nexists.\n\nSo your exploit would just be limited to having someone clone your\nrepository who doesn't already have git-lfs installed into their path,\nwhich I would argue is just as bad.\n\n> Also, config is not versioned, so, right after cloning you wouldn't\n> have this option enabled, so you are always safe after cloning [...]\n\nI know that this has come up in some recent-ish discussions, and I have\nnot been convinced that this makes things any safer.\n\nThanks,\nTaylor\n"}]}