On 1/29/2020 10:30 AM, Johannes Schindelin wrote: > Hi, > > On Tue, 28 Jan 2020, Derrick Stolee wrote: >> I made some progress, at least, in root-causing the issue. >> The problem bisects down to 4dc42c6c1 (mingw: refuse paths >> containing reserved names, 2019-12-21) [1]. CC'ing Dscho. >> >> That commit updates is_valid_win32_path() to fail on these >> paths. We were _already_ calling this method even for paths >> outside the sparse cone, but the method didn't fail for these >> examples. >> >> This means the fix is probably even more complicated: we need >> to not call this method when traversing paths that have the >> skip-worktree bit enabled. This may lead to some tiny >> performance gains when hydrating a very small fraction of a >> very large index. > > Hmm. I am actually not sure that this would be a fix. It is all too easy > for a skip-worktree entry to become checked out (think e.g. a merge > conflict in a cherry-pick, during a three-way merge of a file that is not > in the cone but still needs to be handled). This is a very good point, and something worth investigating. > My preferred solution for this issue would be for the files/directories to > be renamed using `git -c core.protectntfs=false mv <reserved-name> > <non-reserved-name>`. One thing that I realized after root-causing the issue is that now the Linux kernel repository cannot be checked out _at all_ on Windows due to the existence of an aux.c file. Git complains that the path is invalid and does not write a single file to the working directory. At least we _could_ allow someone to create most of the working directory (as we did before) by allowing invalid paths outside the sparse cone. > I think if we try to play extra games with the skip-worktree bit, we risk > opening a vulnerability again. I agree that we need to be _very_ careful with this. Thanks, -Stolee
