* Re: [9front] git/fetch problem in group writable repositories
2021-07-16 5:49 [9front] git/fetch problem in group writable repositories Anthony Martin
@ 2021-07-16 16:05 ` ori
2021-07-16 18:44 ` cinap_lenrek
0 siblings, 1 reply; 6+ messages in thread
From: ori @ 2021-07-16 16:05 UTC (permalink / raw)
To: 9front
Quoth Anthony Martin <ality@pbrane.org>:
> If you create a git repository in a directory that
> is group writable, running git/fetch will leave
> behind an empty .git/objects/pack/fetch.tmp file
> with mode 0644 in a number of scenarios:
>
> - if passed the -l flag
> - if the local repository is up to date
> - if there is a fatal error in fetchpack
>
> If another user in the same group subsequently
> tries to run git/fetch, it will fail since the
> temporary pack file was not removed and cannot
> be overwritten.
>
> The calls to create need to be at least 0660 for
> files and 0770 for directories. It may also make
> sense to create the temporary files with OEXCL
> instead of truncating them if they already exist
> or to use a name that's less likely to be reused
> as in serve.c:/^updatepack.
Good catches.
I think the below patch will solve the problems,
as well as cleaning the code up a little.
Looks good?
> Another thing to think about it is if we should
> grab a lock of some kind before operations that
> will modify a repository that is group writable.
> Or just have a policy of "don't do that".
>
> Thoughts?
Right now, the policy is 'don't do that', but I
am open to changing it.
--- //.git/fs/object/e85aa1089d1d4954aa949cd05c5b6b9c3fca596c/tree/sys/src/cmd/git/fetch.c
+++ sys/src/cmd/git/fetch.c
@@ -5,7 +5,6 @@
char *fetchbranch;
char *upstream = "origin";
-char *packtmp = ".git/objects/pack/fetch.tmp";
int listonly;
int
@@ -111,7 +110,7 @@
for(p=strchr(s+1, '/'); p; p=strchr(p+1, '/')){
*p = 0;
if(access(s, AEXIST) != 0){
- fd = create(s, OREAD, DMDIR | 0755);
+ fd = create(s, OREAD, DMDIR | 0775);
if(fd == -1)
return -1;
close(fd);
@@ -162,13 +161,30 @@
}
}
+void
+fail(char *pack, char *idx, char *msg, ...)
+{
+ char buf[ERRMAX];
+ va_list ap;
+
+ va_start(ap, msg);
+ snprint(buf, sizeof(buf), msg, ap);
+ va_end(ap);
+
+ remove(pack);
+ remove(idx);
+ fprint(2, "%s", buf);
+ exits(buf);
+}
+
int
-fetchpack(Conn *c, int pfd, char *packtmp)
+fetchpack(Conn *c)
{
- char buf[Pktmax], idxtmp[256], *sp[3];
+ char buf[Pktmax], *sp[3];
+ char *packtmp, *idxtmp;
Hash h, *have, *want;
int nref, refsz, first;
- int i, n, req;
+ int i, n, req, pfd;
vlong packsz;
Object *o;
@@ -249,6 +265,15 @@
sysfatal("read: %r");
buf[n] = 0;
+ if((packtmp = smprint(".git/objects/pack/fetch.%d.pack", getpid())) == nil)
+ sysfatal("smprint: %r");
+ if((idxtmp = smprint(".git/objects/idx/fetch.%d.idx", getpid())) == nil)
+ sysfatal("smprint: %r");
+ if(mkoutpath(packtmp) == -1)
+ sysfatal("could not create %s: %r", packtmp);
+ if((pfd = create(packtmp, ORDWR, 0664)) == -1)
+ sysfatal("could not create %s: %r", packtmp);
+
fprint(2, "fetching...\n");
packsz = 0;
while(1){
@@ -259,19 +284,17 @@
sysfatal("fetch packfile: %r");
packsz += n;
}
+
closeconn(c);
if(seek(pfd, 0, 0) == -1)
- sysfatal("packfile seek: %r");
+ fail(packtmp, idxtmp, "packfile seek: %r");
if(checkhash(pfd, packsz, &h) == -1)
- sysfatal("corrupt packfile: %r");
+ fail(packtmp, idxtmp, "corrupt packfile: %r");
close(pfd);
- n = strlen(packtmp) - strlen(".tmp");
- memcpy(idxtmp, packtmp, n);
- memcpy(idxtmp + n, ".idx", strlen(".idx") + 1);
if(indexpack(packtmp, idxtmp, h) == -1)
- sysfatal("could not index fetched pack: %r");
+ fail(packtmp, idxtmp, "could not index fetched pack: %r");
if(rename(packtmp, idxtmp, h) == -1)
- sysfatal("could not rename indexed pack: %r");
+ fail(packtmp, idxtmp, "could not rename indexed pack: %r");
return 0;
}
@@ -287,7 +310,6 @@
void
main(int argc, char **argv)
{
- int pfd;
Conn c;
ARGBEGIN{
@@ -302,14 +324,9 @@
if(argc != 1)
usage();
- if(mkoutpath(packtmp) == -1)
- sysfatal("could not create %s: %r", packtmp);
- if((pfd = create(packtmp, ORDWR, 0644)) == -1)
- sysfatal("could not create %s: %r", packtmp);
-
if(gitconnect(&c, argv[0], "upload") == -1)
sysfatal("could not dial %s: %r", argv[0]);
- if(fetchpack(&c, pfd, packtmp) == -1)
+ if(fetchpack(&c) == -1)
sysfatal("fetch failed: %r");
closeconn(&c);
exits(nil);
^ permalink raw reply [flat|nested] 6+ messages in thread