{"thread":{"id":"55554","subject":"[RFC PATCH] cygwin: disallow backslashes in file names","startedAt":"2021-04-24T21:21:41Z","lastAt":"2021-04-30T00:49:03Z","messageCount":6,"participants":["Adam Dinwoodie","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"422864","messageId":"20210424212117.6165-1-adam@dinwoodie.org","threadId":"55554","inReplyTo":null,"subject":"[RFC PATCH] cygwin: disallow backslashes in file names","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2021-04-24T21:21:17Z","receivedAt":"2021-04-24T21:21:41Z","isPatch":true,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"The backslash character is not a valid part of a file name on Windows,\nso it should not be possible to write files that were unpacked from tree\nobjects where the stored filename contains a backslash character, as it\nwill be interpreted as a directory separator.\n\nThis caused CVE-2019-1354 in mingw, which was fixed by e1d911dd4c\n(\"mingw: disallow backslash characters in tree objects' file names\",\n2019-09-12), however the vulnerability also exists in Cygwin, as while\nCygwin mostly provides a POSIX-like path system, it will also interpret\na backslash as a directory separator in the name of compatibility.\n\nTo avoid the vulnerability, extend the fix for mingw to also apply to\nCygwin.\n\nReported-by: RyotaK <security@ryotak.me>\nHelped-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Adam Dinwoodie <adam@dinwoodie.org>\n---\n\nNotes:\n    The patch to read-cache.c is the one I've applied downstream as the Cygwin Git\n    maintainer to resolve this vulnerability, and I've manually tested that it\n    resolves the vulnerability, so that's the change I'd recommend anyone who needs\n    to build Git on Cygwin themselves take until there's something officially in\n    the Git source code.\n    \n    I'm much less convinced by my approach for the test script.  I definitely think\n    it's worth having a test here, but the test as written still fails, as the test\n    seems to be looking for the error message \"directory not empty\", but running\n    the test on Cygwin produces the error \"cannot create submodule directory d\\a\".\n    I'm not sure why that difference exists, and whether the correct approach would\n    be to (a) ensure the error messages are consistent across platforms or (b) to\n    change the test to expect the appropriate error on the appropriate platform.\n    \n    I'm also not convinced by my approach of adding a \"WINDOWS\" prerequisite to\n    test-lib.sh. I went with this as I couldn't immediately see a way to pass\n    prerequisites on an \"any\" rather than \"all\" basis to test_expect_success, and\n    this would allow us to simplify all the tests that currently have\n    \"!MINGW,!CYGWIN\" as prerequisites, but it still feels a bit clunky to me.\n\n read-cache.c               | 2 +-\n t/t7415-submodule-names.sh | 2 +-\n t/test-lib.sh              | 2 ++\n 3 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 5a907af2fb..b6c13bc04e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -985,7 +985,7 @@ int verify_path(const char *path, unsigned mode)\n \t\t\t\t}\n \t\t\t}\n \t\t\tif (protect_ntfs) {\n-#ifdef GIT_WINDOWS_NATIVE\n+#if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__\n \t\t\t\tif (c == '\\\\')\n \t\t\t\t\treturn 0;\n #endif\ndiff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh\nindex f70368bc2e..6505bc2085 100755\n--- a/t/t7415-submodule-names.sh\n+++ b/t/t7415-submodule-names.sh\n@@ -191,7 +191,7 @@ test_expect_success 'fsck detects corrupt .gitmodules' '\n \t)\n '\n \n-test_expect_success MINGW 'prevent git~1 squatting on Windows' '\n+test_expect_success WINDOWS 'prevent git~1 squatting on Windows' '\n \tgit init squatting &&\n \t(\n \t\tcd squatting &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3dec266221..adaa2db601 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1459,14 +1459,16 @@ case $uname_s in\n \ttest_set_prereq NATIVE_CRLF\n \ttest_set_prereq SED_STRIPS_CR\n \ttest_set_prereq GREP_STRIPS_CR\n+\ttest_set_prereq WINDOWS\n \tGIT_TEST_CMP=mingw_test_cmp\n \t;;\n *CYGWIN*)\n \ttest_set_prereq POSIXPERM\n \ttest_set_prereq EXECKEEPSPID\n \ttest_set_prereq CYGWIN\n \ttest_set_prereq SED_STRIPS_CR\n \ttest_set_prereq GREP_STRIPS_CR\n+\ttest_set_prereq WINDOWS\n \t;;\n *)\n \ttest_set_prereq POSIXPERM\n-- \n2.31.1\n\n"},{"id":"422924","messageId":"nycvar.QRO.7.76.6.2104250413320.54@tvgsbejvaqbjf.bet","threadId":"55554","inReplyTo":"20210424212117.6165-1-adam@dinwoodie.org","subject":"Re: [RFC PATCH] cygwin: disallow backslashes in file names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-04-25T02:22:03Z","receivedAt":"2021-04-26T14:09:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Adam,\n\nOn Sat, 24 Apr 2021, Adam Dinwoodie wrote:\n\n> The backslash character is not a valid part of a file name on Windows,\n> so it should not be possible to write files that were unpacked from tree\n> objects where the stored filename contains a backslash character, as it\n> will be interpreted as a directory separator.\n>\n> This caused CVE-2019-1354 in mingw, which was fixed by e1d911dd4c\n> (\"mingw: disallow backslash characters in tree objects' file names\",\n> 2019-09-12), however the vulnerability also exists in Cygwin, as while\n> Cygwin mostly provides a POSIX-like path system, it will also interpret\n> a backslash as a directory separator in the name of compatibility.\n>\n> To avoid the vulnerability, extend the fix for mingw to also apply to\n> Cygwin.\n>\n> Reported-by: RyotaK <security@ryotak.me>\n> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>\n> ---\n>\n> Notes:\n>     The patch to read-cache.c is the one I've applied downstream as the Cygwin Git\n>     maintainer to resolve this vulnerability, and I've manually tested that it\n>     resolves the vulnerability, so that's the change I'd recommend anyone who needs\n>     to build Git on Cygwin themselves take until there's something officially in\n>     the Git source code.\n>\n>     I'm much less convinced by my approach for the test script.  I definitely think\n>     it's worth having a test here, but the test as written still fails, as the test\n>     seems to be looking for the error message \"directory not empty\", but running\n>     the test on Cygwin produces the error \"cannot create submodule directory d\\a\".\n>     I'm not sure why that difference exists, and whether the correct approach would\n>     be to (a) ensure the error messages are consistent across platforms or (b) to\n>     change the test to expect the appropriate error on the appropriate platform.\n\nWasn't there something in Cygwin that _allowed_ backslashes as file name\ncharacters? I vaguely remember that the ASCII characters forbidden by\nWindows were mapped into some \"private page\".\n\nMaybe that is responsible for the difference here?\n\n>     I'm also not convinced by my approach of adding a \"WINDOWS\" prerequisite to\n>     test-lib.sh. I went with this as I couldn't immediately see a way to pass\n>     prerequisites on an \"any\" rather than \"all\" basis to test_expect_success, and\n>     this would allow us to simplify all the tests that currently have\n>     \"!MINGW,!CYGWIN\" as prerequisites, but it still feels a bit clunky to me.\n\nRight, the only way I could think of it would be\n\n\ttest_lazy_prereq 'test_have_prereq MINGW || test_have_prereq CYGWIN'\n\nYour approach looks fine to me, though.\n\nCiao,\nDscho\n\n>\n>  read-cache.c               | 2 +-\n>  t/t7415-submodule-names.sh | 2 +-\n>  t/test-lib.sh              | 2 ++\n>  3 files changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index 5a907af2fb..b6c13bc04e 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -985,7 +985,7 @@ int verify_path(const char *path, unsigned mode)\n>  \t\t\t\t}\n>  \t\t\t}\n>  \t\t\tif (protect_ntfs) {\n> -#ifdef GIT_WINDOWS_NATIVE\n> +#if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__\n>  \t\t\t\tif (c == '\\\\')\n>  \t\t\t\t\treturn 0;\n>  #endif\n> diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh\n> index f70368bc2e..6505bc2085 100755\n> --- a/t/t7415-submodule-names.sh\n> +++ b/t/t7415-submodule-names.sh\n> @@ -191,7 +191,7 @@ test_expect_success 'fsck detects corrupt .gitmodules' '\n>  \t)\n>  '\n>\n> -test_expect_success MINGW 'prevent git~1 squatting on Windows' '\n> +test_expect_success WINDOWS 'prevent git~1 squatting on Windows' '\n>  \tgit init squatting &&\n>  \t(\n>  \t\tcd squatting &&\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 3dec266221..adaa2db601 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -1459,14 +1459,16 @@ case $uname_s in\n>  \ttest_set_prereq NATIVE_CRLF\n>  \ttest_set_prereq SED_STRIPS_CR\n>  \ttest_set_prereq GREP_STRIPS_CR\n> +\ttest_set_prereq WINDOWS\n>  \tGIT_TEST_CMP=mingw_test_cmp\n>  \t;;\n>  *CYGWIN*)\n>  \ttest_set_prereq POSIXPERM\n>  \ttest_set_prereq EXECKEEPSPID\n>  \ttest_set_prereq CYGWIN\n>  \ttest_set_prereq SED_STRIPS_CR\n>  \ttest_set_prereq GREP_STRIPS_CR\n> +\ttest_set_prereq WINDOWS\n>  \t;;\n>  *)\n>  \ttest_set_prereq POSIXPERM\n> --\n> 2.31.1\n>\n>\n"},{"id":"423122","messageId":"CA+kUOamYmFcKA+_on83=EbitvL4FQo9teMEbRHsQ=xo2ave1yQ@mail.gmail.com","threadId":"55554","inReplyTo":"CA+kUOan3vk1zJezpieRhKwZ8gsYrCxDBefkXJ1fUC61O+gb12A@mail.gmail.com","subject":"Re: [RFC PATCH] cygwin: disallow backslashes in file names","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2021-04-27T19:22:49Z","receivedAt":"2021-04-27T19:23:29Z","isPatch":true,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"[Re-adding the previous Cc list that I'd failed to copy on my previous\nemail, sorry!]\n\nOn Mon, 26 Apr 2021 at 20:56, Adam Dinwoodie wrote:\n>\n> On Mon, 26 Apr 2021 at 15:08, Johannes Schindelin wrote:\n> >\n> > Hi Adam,\n> >\n> > On Sat, 24 Apr 2021, Adam Dinwoodie wrote:\n> > > Notes:\n> > >     The patch to read-cache.c is the one I've applied downstream as the Cygwin Git\n> > >     maintainer to resolve this vulnerability, and I've manually tested that it\n> > >     resolves the vulnerability, so that's the change I'd recommend anyone who needs\n> > >     to build Git on Cygwin themselves take until there's something officially in\n> > >     the Git source code.\n> > >\n> > >     I'm much less convinced by my approach for the test script.  I definitely think\n> > >     it's worth having a test here, but the test as written still fails, as the test\n> > >     seems to be looking for the error message \"directory not empty\", but running\n> > >     the test on Cygwin produces the error \"cannot create submodule directory d\\a\".\n> > >     I'm not sure why that difference exists, and whether the correct approach would\n> > >     be to (a) ensure the error messages are consistent across platforms or (b) to\n> > >     change the test to expect the appropriate error on the appropriate platform.\n> >\n> > Wasn't there something in Cygwin that _allowed_ backslashes as file name\n> > characters? I vaguely remember that the ASCII characters forbidden by\n> > Windows were mapped into some \"private page\".\n> >\n> > Maybe that is responsible for the difference here?\n>\n> So there is special handling of a bunch of characters like \":\" that\n> are valid as parts of filenames on most *nix systems, but which aren't\n> valid on Windows, by substituting them for characters in the Unicode\n> \"private use area\" space. Backslash isn't one of those characters,\n> though; quoting\n> https://cygwin.com/cygwin-ug-net/using-specialnames.html (which I just\n> checked myself to be sure): \"The backslash has to be exempt from this\n> conversion, because Cygwin accepts Win32 filenames including\n> backslashes as path separators on input.\"\n>\n> Which is not to say this special handling _isn't_ the cause of the\n> difference here, but it's not so simple as that. If nobody spots an\n> explanation I've missed, I'll start digging into the code and strace\n> to work out exactly what's causing the difference in behaviour.\n\nI've worked out what's going wrong here: the \"prevent git~1 squatting\non Windows\" test is actually testing a selection of different Windows\npath oddities, which are handled differently between Git for Windows\nand Cygwin Git. The specific behaviour here is the handling of a\ndirectory called \"d.\"; Git for Windows (I assume in the MSYS2 layer)\nfollows the standard Windows convention of treating \"d.\" and \"d\" as\nidentical filenames, while Cygwin sticks to its general design\nphilosophy of mostly emulating *nix systems, allowing objects with\nboth filenames to exist in the same directory (and causing pain for\nmost non-Cygwin applications that try to interact with them).\n\nEssentially this test is checking a bunch of different oddities about\npath handling on Windows. Some things – such as handling backslashes –\nare common to both Cygwin and MSYS2; some – such as handling trailing\nperiods – aren't. So I expect the solution here will be to have\nseparate tests for (a) Git for Windows, (b) Cygwin Git, and (c) common\n\nbehaviour.\n\n> > >     I'm also not convinced by my approach of adding a \"WINDOWS\" prerequisite to\n> > >     test-lib.sh. I went with this as I couldn't immediately see a way to pass\n> > >     prerequisites on an \"any\" rather than \"all\" basis to test_expect_success, and\n> > >     this would allow us to simplify all the tests that currently have\n> > >     \"!MINGW,!CYGWIN\" as prerequisites, but it still feels a bit clunky to me.\n> >\n> > Right, the only way I could think of it would be\n> >\n> >         test_lazy_prereq 'test_have_prereq MINGW || test_have_prereq CYGWIN'\n> >\n> > Your approach looks fine to me, though.\n>\n> Grand, okay. I'll stick with that for now, then, and follow up with a\n> patch to tidy up the other prerequisites at some point in the future.\n>\n> Adam\n"},{"id":"423195","messageId":"nycvar.QRO.7.76.6.2104280226530.54@tvgsbejvaqbjf.bet","threadId":"55554","inReplyTo":"CA+kUOamYmFcKA+_on83=EbitvL4FQo9teMEbRHsQ=xo2ave1yQ@mail.gmail.com","subject":"Re: [RFC PATCH] cygwin: disallow backslashes in file names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-04-28T00:27:18Z","receivedAt":"2021-04-28T14:12:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Adam,\n\nOn Tue, 27 Apr 2021, Adam Dinwoodie wrote:\n\n> On Mon, 26 Apr 2021 at 20:56, Adam Dinwoodie wrote:\n> >\n> > On Mon, 26 Apr 2021 at 15:08, Johannes Schindelin wrote:\n> > >\n> > > Hi Adam,\n> > >\n> > > On Sat, 24 Apr 2021, Adam Dinwoodie wrote:\n> > > > Notes:\n> > > >     The patch to read-cache.c is the one I've applied downstream as the Cygwin Git\n> > > >     maintainer to resolve this vulnerability, and I've manually tested that it\n> > > >     resolves the vulnerability, so that's the change I'd recommend anyone who needs\n> > > >     to build Git on Cygwin themselves take until there's something officially in\n> > > >     the Git source code.\n> > > >\n> > > >     I'm much less convinced by my approach for the test script.  I definitely think\n> > > >     it's worth having a test here, but the test as written still fails, as the test\n> > > >     seems to be looking for the error message \"directory not empty\", but running\n> > > >     the test on Cygwin produces the error \"cannot create submodule directory d\\a\".\n> > > >     I'm not sure why that difference exists, and whether the correct approach would\n> > > >     be to (a) ensure the error messages are consistent across platforms or (b) to\n> > > >     change the test to expect the appropriate error on the appropriate platform.\n> > >\n> > > Wasn't there something in Cygwin that _allowed_ backslashes as file name\n> > > characters? I vaguely remember that the ASCII characters forbidden by\n> > > Windows were mapped into some \"private page\".\n> > >\n> > > Maybe that is responsible for the difference here?\n> >\n> > So there is special handling of a bunch of characters like \":\" that\n> > are valid as parts of filenames on most *nix systems, but which aren't\n> > valid on Windows, by substituting them for characters in the Unicode\n> > \"private use area\" space. Backslash isn't one of those characters,\n> > though; quoting\n> > https://cygwin.com/cygwin-ug-net/using-specialnames.html (which I just\n> > checked myself to be sure): \"The backslash has to be exempt from this\n> > conversion, because Cygwin accepts Win32 filenames including\n> > backslashes as path separators on input.\"\n> >\n> > Which is not to say this special handling _isn't_ the cause of the\n> > difference here, but it's not so simple as that. If nobody spots an\n> > explanation I've missed, I'll start digging into the code and strace\n> > to work out exactly what's causing the difference in behaviour.\n>\n> I've worked out what's going wrong here: the \"prevent git~1 squatting\n> on Windows\" test is actually testing a selection of different Windows\n> path oddities, which are handled differently between Git for Windows\n> and Cygwin Git. The specific behaviour here is the handling of a\n> directory called \"d.\"; Git for Windows (I assume in the MSYS2 layer)\n> follows the standard Windows convention of treating \"d.\" and \"d\" as\n> identical filenames, while Cygwin sticks to its general design\n> philosophy of mostly emulating *nix systems, allowing objects with\n> both filenames to exist in the same directory (and causing pain for\n> most non-Cygwin applications that try to interact with them).\n>\n> Essentially this test is checking a bunch of different oddities about\n> path handling on Windows. Some things – such as handling backslashes –\n> are common to both Cygwin and MSYS2; some – such as handling trailing\n> periods – aren't. So I expect the solution here will be to have\n> separate tests for (a) Git for Windows, (b) Cygwin Git, and (c) common\n> behaviour.\n\nAh, that would explain things. Thank you so much for digging!\n\nCiao,\nDscho\n\n>\n> > > >     I'm also not convinced by my approach of adding a \"WINDOWS\" prerequisite to\n> > > >     test-lib.sh. I went with this as I couldn't immediately see a way to pass\n> > > >     prerequisites on an \"any\" rather than \"all\" basis to test_expect_success, and\n> > > >     this would allow us to simplify all the tests that currently have\n> > > >     \"!MINGW,!CYGWIN\" as prerequisites, but it still feels a bit clunky to me.\n> > >\n> > > Right, the only way I could think of it would be\n> > >\n> > >         test_lazy_prereq 'test_have_prereq MINGW || test_have_prereq CYGWIN'\n> > >\n> > > Your approach looks fine to me, though.\n> >\n> > Grand, okay. I'll stick with that for now, then, and follow up with a\n> > patch to tidy up the other prerequisites at some point in the future.\n> >\n> > Adam\n>\n"},{"id":"423297","messageId":"20210429201144.8936-1-adam@dinwoodie.org","threadId":"55554","inReplyTo":"20210424212117.6165-1-adam@dinwoodie.org","subject":"[PATCH] cygwin: disallow backslashes in file names","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2021-04-29T20:11:44Z","receivedAt":"2021-04-29T20:12:32Z","isPatch":true,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"The backslash character is not a valid part of a file name on Windows.\nIf, in Windows, Git attempts to write a file that has a backslash\ncharacter in the filename, it will be incorrectly interpreted as a\ndirectory separator.\n\nThis caused CVE-2019-1354 in MinGW, as this behaviour can be manipulated\nto cause the checkout to write to files it ought not write to, such as\nadding code to the .git/hooks directory.  This was fixed by e1d911dd4c\n(mingw: disallow backslash characters in tree objects' file names,\n2019-09-12).  However, the vulnerability also exists in Cygwin: while\nCygwin mostly provides a POSIX-like path system, it will still interpret\na backslash as a directory separator.\n\nTo avoid this vulnerability, CVE-2021-29468, extend the previous fix to\nalso apply to Cygwin.\n\nSimilarly, extend the test case added by the previous version of the\ncommit.  The test suite doesn't have an easy way to say \"run this test\nif in MinGW or Cygwin\", so add a new test prerequisite that covers both.\n\nAs well as checking behaviour in the presence of paths containing\nbackslashes, the existing test also checks behaviour in the presence of\npaths that differ only by the presence of a trailing \".\".  MinGW follows\nnormal Windows application behaviour and treats them as the same path,\nbut Cygwin more closely emulates *nix systems (at the expense of\ncompatibility with native Windows applications) and will create and\ndistinguish between such paths.  Gate the relevant bit of that test\naccordingly.\n\nReported-by: RyotaK <security@ryotak.me>\nHelped-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Adam Dinwoodie <adam@dinwoodie.org>\n---\n read-cache.c               |  2 +-\n t/test-lib.sh              |  2 ++\n t/t7415-submodule-names.sh | 13 ++++++++-----\n 3 files changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 5a907af2fb..b6c13bc04e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -985,7 +985,7 @@ int verify_path(const char *path, unsigned mode)\n \t\t\t\t}\n \t\t\t}\n \t\t\tif (protect_ntfs) {\n-#ifdef GIT_WINDOWS_NATIVE\n+#if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__\n \t\t\t\tif (c == '\\\\')\n \t\t\t\t\treturn 0;\n #endif\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex d3f6af6a65..e84b8c87f9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1457,14 +1457,16 @@ case $uname_s in\n \ttest_set_prereq NATIVE_CRLF\n \ttest_set_prereq SED_STRIPS_CR\n \ttest_set_prereq GREP_STRIPS_CR\n+\ttest_set_prereq WINDOWS\n \tGIT_TEST_CMP=mingw_test_cmp\n \t;;\n *CYGWIN*)\n \ttest_set_prereq POSIXPERM\n \ttest_set_prereq EXECKEEPSPID\n \ttest_set_prereq CYGWIN\n \ttest_set_prereq SED_STRIPS_CR\n \ttest_set_prereq GREP_STRIPS_CR\n+\ttest_set_prereq WINDOWS\n \t;;\n *)\n \ttest_set_prereq POSIXPERM\ndiff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh\nindex f70368bc2e..6bf098a6be 100755\n--- a/t/t7415-submodule-names.sh\n+++ b/t/t7415-submodule-names.sh\n@@ -191,7 +191,7 @@ test_expect_success 'fsck detects corrupt .gitmodules' '\n \t)\n '\n \n-test_expect_success MINGW 'prevent git~1 squatting on Windows' '\n+test_expect_success WINDOWS 'prevent git~1 squatting on Windows' '\n \tgit init squatting &&\n \t(\n \t\tcd squatting &&\n@@ -219,10 +219,13 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' '\n \t\ttest_tick &&\n \t\tgit -c core.protectNTFS=false commit -m \"module\"\n \t) &&\n-\ttest_must_fail git -c core.protectNTFS=false \\\n-\t\tclone --recurse-submodules squatting squatting-clone 2>err &&\n-\ttest_i18ngrep -e \"directory not empty\" -e \"not an empty directory\" err &&\n-\t! grep gitdir squatting-clone/d/a/git~2\n+\tif test_have_prereq MINGW\n+\tthen\n+\t\ttest_must_fail git -c core.protectNTFS=false \\\n+\t\t\tclone --recurse-submodules squatting squatting-clone 2>err &&\n+\t\ttest_i18ngrep -e \"directory not empty\" -e \"not an empty directory\" err &&\n+\t\t! grep gitdir squatting-clone/d/a/git~2\n+\tfi\n '\n \n test_expect_success 'git dirs of sibling submodules must not be nested' '\n-- \n2.31.1\n\n"},{"id":"423309","messageId":"xmqqsg38egw6.fsf@gitster.g","threadId":"55554","inReplyTo":"20210429201144.8936-1-adam@dinwoodie.org","subject":"Re: [PATCH] cygwin: disallow backslashes in file names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-30T00:48:57Z","receivedAt":"2021-04-30T00:49:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Dinwoodie <adam@dinwoodie.org> writes:\n\n> The backslash character is not a valid part of a file name on Windows.\n> If, in Windows, Git attempts to write a file that has a backslash\n> character in the filename, it will be incorrectly interpreted as a\n> directory separator.\n>\n> This caused CVE-2019-1354 in MinGW, as this behaviour can be manipulated\n> to cause the checkout to write to files it ought not write to, such as\n> adding code to the .git/hooks directory.  This was fixed by e1d911dd4c\n> (mingw: disallow backslash characters in tree objects' file names,\n> 2019-09-12).  However, the vulnerability also exists in Cygwin: while\n> Cygwin mostly provides a POSIX-like path system, it will still interpret\n> a backslash as a directory separator.\n>\n> To avoid this vulnerability, CVE-2021-29468, extend the previous fix to\n> also apply to Cygwin.\n>\n> Similarly, extend the test case added by the previous version of the\n> commit.  The test suite doesn't have an easy way to say \"run this test\n> if in MinGW or Cygwin\", so add a new test prerequisite that covers both.\n>\n> As well as checking behaviour in the presence of paths containing\n> backslashes, the existing test also checks behaviour in the presence of\n> paths that differ only by the presence of a trailing \".\".  MinGW follows\n> normal Windows application behaviour and treats them as the same path,\n> but Cygwin more closely emulates *nix systems (at the expense of\n> compatibility with native Windows applications) and will create and\n> distinguish between such paths.  Gate the relevant bit of that test\n> accordingly.\n>\n> Reported-by: RyotaK <security@ryotak.me>\n> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>\n> ---\n\nThanks, all.  Will queue.\n\n>  read-cache.c               |  2 +-\n>  t/test-lib.sh              |  2 ++\n>  t/t7415-submodule-names.sh | 13 ++++++++-----\n>  3 files changed, 11 insertions(+), 6 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index 5a907af2fb..b6c13bc04e 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -985,7 +985,7 @@ int verify_path(const char *path, unsigned mode)\n>  \t\t\t\t}\n>  \t\t\t}\n>  \t\t\tif (protect_ntfs) {\n> -#ifdef GIT_WINDOWS_NATIVE\n> +#if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__\n>  \t\t\t\tif (c == '\\\\')\n>  \t\t\t\t\treturn 0;\n>  #endif\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index d3f6af6a65..e84b8c87f9 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -1457,14 +1457,16 @@ case $uname_s in\n>  \ttest_set_prereq NATIVE_CRLF\n>  \ttest_set_prereq SED_STRIPS_CR\n>  \ttest_set_prereq GREP_STRIPS_CR\n> +\ttest_set_prereq WINDOWS\n>  \tGIT_TEST_CMP=mingw_test_cmp\n>  \t;;\n>  *CYGWIN*)\n>  \ttest_set_prereq POSIXPERM\n>  \ttest_set_prereq EXECKEEPSPID\n>  \ttest_set_prereq CYGWIN\n>  \ttest_set_prereq SED_STRIPS_CR\n>  \ttest_set_prereq GREP_STRIPS_CR\n> +\ttest_set_prereq WINDOWS\n>  \t;;\n>  *)\n>  \ttest_set_prereq POSIXPERM\n> diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh\n> index f70368bc2e..6bf098a6be 100755\n> --- a/t/t7415-submodule-names.sh\n> +++ b/t/t7415-submodule-names.sh\n> @@ -191,7 +191,7 @@ test_expect_success 'fsck detects corrupt .gitmodules' '\n>  \t)\n>  '\n>  \n> -test_expect_success MINGW 'prevent git~1 squatting on Windows' '\n> +test_expect_success WINDOWS 'prevent git~1 squatting on Windows' '\n>  \tgit init squatting &&\n>  \t(\n>  \t\tcd squatting &&\n> @@ -219,10 +219,13 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' '\n>  \t\ttest_tick &&\n>  \t\tgit -c core.protectNTFS=false commit -m \"module\"\n>  \t) &&\n> -\ttest_must_fail git -c core.protectNTFS=false \\\n> -\t\tclone --recurse-submodules squatting squatting-clone 2>err &&\n> -\ttest_i18ngrep -e \"directory not empty\" -e \"not an empty directory\" err &&\n> -\t! grep gitdir squatting-clone/d/a/git~2\n> +\tif test_have_prereq MINGW\n> +\tthen\n> +\t\ttest_must_fail git -c core.protectNTFS=false \\\n> +\t\t\tclone --recurse-submodules squatting squatting-clone 2>err &&\n> +\t\ttest_i18ngrep -e \"directory not empty\" -e \"not an empty directory\" err &&\n> +\t\t! grep gitdir squatting-clone/d/a/git~2\n> +\tfi\n>  '\n>  \n>  test_expect_success 'git dirs of sibling submodules must not be nested' '\n"}]}