# [PATCH] cygwin: set write permission before unlink 3 messages from 2011-06-29 to 2011-06-29. Participants: Rei Thiessen, Christof Krüger. Thread: https://gitlist.dev/t/27726 ## Rei Thiessen, 2011-06-29 07:18 Subject: [PATCH] cygwin: set write permission before unlink Message-ID: <1309331898-32247-1-git-send-email-rei.thiessen@gmail.com> URL: https://gitlist.dev/e/1309331898-32247-1-git-send-email-rei.thiessen%40gmail.com ``` On Cygwin 1.7, if filesystems are mounted with the "noacl" option, files without write permission (in particular, the temp files created in write_loose_object()) have their "read-only" flags set in NTFS. "read-only" files can't be unlinked. For Cygwin, set write permissions on files that are about to be unlinked. Signed-off-by: Rei Thiessen --- compat/cygwin.c | 7 +++++++ compat/cygwin.h | 3 +++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/compat/cygwin.c b/compat/cygwin.c index b4a51b9..f40ee6d 100644 --- a/compat/cygwin.c +++ b/compat/cygwin.c @@ -141,3 +141,10 @@ static int cygwin_lstat_stub(const char *file_name, struct stat *buf) stat_fn_t cygwin_stat_fn = cygwin_stat_stub; stat_fn_t cygwin_lstat_fn = cygwin_lstat_stub; +#undef unlink +int cygwin_unlink(const char *pathname) +{ + /* "read-only" files can't be unlinked */ + chmod(pathname, 0666); + return unlink(pathname); +} diff --git a/compat/cygwin.h b/compat/cygwin.h index a3229f5..aa2ba3e 100644 --- a/compat/cygwin.h +++ b/compat/cygwin.h @@ -7,3 +7,6 @@ extern stat_fn_t cygwin_lstat_fn; #define stat(path, buf) (*cygwin_stat_fn)(path, buf) #define lstat(path, buf) (*cygwin_lstat_fn)(path, buf) + +int cygwin_unlink(const char *pathname); +#define unlink cygwin_unlink -- 1.7.4.1 ``` ## Christof Krüger, 2011-06-29 14:31 Subject: Re: [PATCH] cygwin: set write permission before unlink Message-ID: <09c0b1900a67bd1f701c0b23954a34ab.squirrel@mail.localhost.li> URL: https://gitlist.dev/e/09c0b1900a67bd1f701c0b23954a34ab.squirrel%40mail.localhost.li In-Reply-To: <1309331898-32247-1-git-send-email-rei.thiessen@gmail.com> ``` > +#undef unlink > +int cygwin_unlink(const char *pathname) > +{ > + /* "read-only" files can't be unlinked */ > + chmod(pathname, 0666); > + return unlink(pathname); > +} I've no idea on how cygwin maps file permissions to the underlying filesystem, but the above raised my attention. Doesn't chmodding the file to 0666 open a small windows where "group" and "other" users have read access to the file? This might be unwanted by the user and could be exploited by some attacker listening for changes on that file. Or am I too paranoid? Regards, Chris ``` ## Rei Thiessen, 2011-06-29 18:48 Subject: Re: [PATCH] cygwin: set write permission before unlink Message-ID: URL: https://gitlist.dev/e/BANLkTincfBvDBYYLGJe-m5hknvCVdmMUww%40mail.gmail.com In-Reply-To: <09c0b1900a67bd1f701c0b23954a34ab.squirrel@mail.localhost.li> ``` Another point of concern is that the user might have specifically set the read-only flag only certain files to protect them from changes/deletion, but after the patch, git can delete them with impunity. But then, a file's permission isn't supposed to matter to unlink() anyways. Interestingly, Cygwin's packaged unlink command-line utility will delete read-only files, so Cygwin's attempt to fake permissions through the read-only flag when a filesystem is mounted with "noacl" seems to be inconsistent. I'll leave this issue up to Cygwin's package maintainer for git. Regards, Rei 2011/6/29 Christof Krüger : >> +#undef unlink >> +int cygwin_unlink(const char *pathname) >> +{ >> +     /* "read-only" files can't be unlinked */ >> +     chmod(pathname, 0666); >> +     return unlink(pathname); >> +} > > I've no idea on how cygwin maps file permissions to the underlying > filesystem, but the above raised my attention. Doesn't chmodding the file > to 0666 open a small windows where "group" and "other" users have read > access to the file? This might be unwanted by the user and could be > exploited by some attacker listening for changes on that file. > Or am I too paranoid? > > Regards, >  Chris > > ```