{"thread":{"id":"27726","subject":"[PATCH] cygwin: set write permission before unlink","startedAt":"2011-06-29T07:18:18Z","lastAt":"2011-06-29T18:48:40Z","messageCount":3,"participants":["Rei Thiessen","Christof Krüger"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"170662","messageId":"1309331898-32247-1-git-send-email-rei.thiessen@gmail.com","threadId":"27726","inReplyTo":null,"subject":"[PATCH] cygwin: set write permission before unlink","fromName":"Rei Thiessen","fromEmail":"rei.thiessen@gmail.com","sentAt":"2011-06-29T07:18:18Z","receivedAt":"2011-06-29T07:18:18Z","isPatch":true,"sender":{"key":"rei.thiessen@gmail.com","avatar":null},"body":"On Cygwin 1.7, if filesystems are mounted with the \"noacl\" option,\nfiles without write permission (in particular, the temp files created\nin write_loose_object()) have their \"read-only\" flags set in NTFS.\n\"read-only\" files can't be unlinked.\n\nFor Cygwin, set write permissions on files that are about to be unlinked.\n\nSigned-off-by: Rei Thiessen <rei.thiessen@gmail.com>\n---\n compat/cygwin.c |    7 +++++++\n compat/cygwin.h |    3 +++\n 2 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex b4a51b9..f40ee6d 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -141,3 +141,10 @@ static int cygwin_lstat_stub(const char *file_name, struct stat *buf)\n stat_fn_t cygwin_stat_fn = cygwin_stat_stub;\n stat_fn_t cygwin_lstat_fn = cygwin_lstat_stub;\n \n+#undef unlink\n+int cygwin_unlink(const char *pathname)\n+{\n+\t/* \"read-only\" files can't be unlinked */\n+\tchmod(pathname, 0666);\n+\treturn unlink(pathname);\n+}\ndiff --git a/compat/cygwin.h b/compat/cygwin.h\nindex a3229f5..aa2ba3e 100644\n--- a/compat/cygwin.h\n+++ b/compat/cygwin.h\n@@ -7,3 +7,6 @@ extern stat_fn_t cygwin_lstat_fn;\n \n #define stat(path, buf) (*cygwin_stat_fn)(path, buf)\n #define lstat(path, buf) (*cygwin_lstat_fn)(path, buf)\n+\n+int cygwin_unlink(const char *pathname);\n+#define unlink cygwin_unlink\n-- \n1.7.4.1\n"},{"id":"170668","messageId":"09c0b1900a67bd1f701c0b23954a34ab.squirrel@mail.localhost.li","threadId":"27726","inReplyTo":"1309331898-32247-1-git-send-email-rei.thiessen@gmail.com","subject":"Re: [PATCH] cygwin: set write permission before unlink","fromName":"Christof Krüger","fromEmail":"git@christof-krueger.de","sentAt":"2011-06-29T14:31:20Z","receivedAt":"2011-06-29T14:31:20Z","isPatch":true,"sender":{"key":"git@christof-krueger.de","avatar":null},"body":"> +#undef unlink\n> +int cygwin_unlink(const char *pathname)\n> +{\n> +\t/* \"read-only\" files can't be unlinked */\n> +\tchmod(pathname, 0666);\n> +\treturn unlink(pathname);\n> +}\n\nI've no idea on how cygwin maps file permissions to the underlying\nfilesystem, but the above raised my attention. Doesn't chmodding the file\nto 0666 open a small windows where \"group\" and \"other\" users have read\naccess to the file? This might be unwanted by the user and could be\nexploited by some attacker listening for changes on that file.\nOr am I too paranoid?\n\nRegards,\n  Chris\n"},{"id":"170679","messageId":"BANLkTincfBvDBYYLGJe-m5hknvCVdmMUww@mail.gmail.com","threadId":"27726","inReplyTo":"09c0b1900a67bd1f701c0b23954a34ab.squirrel@mail.localhost.li","subject":"Re: [PATCH] cygwin: set write permission before unlink","fromName":"Rei Thiessen","fromEmail":"rei.thiessen@gmail.com","sentAt":"2011-06-29T18:48:40Z","receivedAt":"2011-06-29T18:48:40Z","isPatch":true,"sender":{"key":"rei.thiessen@gmail.com","avatar":null},"body":"Another point of concern is that the user might have specifically set\nthe read-only flag only certain files\nto protect them from changes/deletion, but after the patch, git can\ndelete them with impunity.\nBut then, a file's permission isn't supposed to matter to unlink() anyways.\nInterestingly, Cygwin's packaged unlink command-line utility will\ndelete read-only files,\nso Cygwin's attempt to fake permissions through the read-only flag\nwhen a filesystem is mounted with \"noacl\"\nseems to be inconsistent.\n\nI'll leave this issue up to Cygwin's package maintainer for git.\n\nRegards,\nRei\n\n\n2011/6/29 Christof Krüger <git@christof-krueger.de>:\n>> +#undef unlink\n>> +int cygwin_unlink(const char *pathname)\n>> +{\n>> +     /* \"read-only\" files can't be unlinked */\n>> +     chmod(pathname, 0666);\n>> +     return unlink(pathname);\n>> +}\n>\n> I've no idea on how cygwin maps file permissions to the underlying\n> filesystem, but the above raised my attention. Doesn't chmodding the file\n> to 0666 open a small windows where \"group\" and \"other\" users have read\n> access to the file? This might be unwanted by the user and could be\n> exploited by some attacker listening for changes on that file.\n> Or am I too paranoid?\n>\n> Regards,\n>  Chris\n>\n>\n"}]}