[[e-mail]: [Full-disclosure] [ZH2005-02SA] Insecure tmp file creation in Wine]

[Marcus Meissner]

Hi,
> From full-disclosure, a security mailing list.

Fix seems trivial, just use 0600.

(untested, uncompiled ;)

Ciao, Marcus

Changelog:
	Temporary registries exported only user read/writeable to
	avoid information leaks.

Index: server/registry.c
===================================================================
RCS file: /home/wine/wine/server/registry.c,v
retrieving revision 1.63
diff -u -r1.63 registry.c
--- server/registry.c	10 Mar 2005 11:18:31 -0000	1.63
+++ server/registry.c	14 Mar 2005 16:38:54 -0000
@@ -1610,7 +1610,7 @@
     for (;;)
     {
         sprintf( p, "reg%lx%04x.tmp", (long) getpid(), count++ );
-        if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0666 )) != -1) break;
+        if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0600 )) != -1) break;
         if (errno != EEXIST) goto done;
         close( fd );
     }
Index: misc/registry.c
===================================================================
RCS file: /home/wine/wine/misc/registry.c,v
retrieving revision 1.149
diff -u -r1.149 registry.c
--- misc/registry.c	25 Aug 2004 00:48:47 -0000	1.149
+++ misc/registry.c	14 Mar 2005 16:38:55 -0000
@@ -1227,7 +1227,7 @@
     ret = _xmalloc(50);
     for (count = 0;;) {
         sprintf(ret,"/tmp/reg%lx%04x.tmp",(long)getpid(),count++);
-        if ((tmp_fd = open(ret,O_CREAT | O_EXCL | O_WRONLY,0666)) != -1) break;
+        if ((tmp_fd = open(ret,O_CREAT | O_EXCL | O_WRONLY,0600)) != -1) break;
         if (errno != EEXIST) {
             ERR("Unexpected error while open() call: %s\n",strerror(errno));
             free(ret);

[Alexandre Julliard]

Marcus Meissner <meissner(a)suse.de> writes:
> --- server/registry.c	10 Mar 2005 11:18:31 -0000	1.63
> +++ server/registry.c	14 Mar 2005 16:38:54 -0000
> @@ -1610,7 +1610,7 @@
>      for (;;)
>      {
>          sprintf( p, "reg%lx%04x.tmp", (long) getpid(), count++ );
> -        if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0666 )) != -1) break;
> +        if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0600 )) != -1) break;

That one is not in /tmp, it's in the user's home directory.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Peter Bortas]

Alexandre Julliard <julliard(a)winehq.org> writes:
> Marcus Meissner <meissner(a)suse.de> writes:
> > --- server/registry.c	10 Mar 2005 11:18:31 -0000	1.63
> > +++ server/registry.c	14 Mar 2005 16:38:54 -0000
> > @@ -1610,7 +1610,7 @@
> >      for (;;)
> >      {
> >          sprintf( p, "reg%lx%04x.tmp", (long) getpid(), count++ );
> > -        if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0666 )) != -1) break;
> > +        if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0600 )) != -1) break;
>
> That one is not in /tmp, it's in the user's home directory.

Home directories are group readable on many sites, so to prevent
information leakage 0600 would still be prudent.

-- 
Peter Bortas                  http://peter.bortas.org

[Alexandre Julliard]

Peter Bortas <peter(a)bortas.org> writes:
> Alexandre Julliard <julliard(a)winehq.org> writes:
> > That one is not in /tmp, it's in the user's home directory.
>
> Home directories are group readable on many sites, so to prevent
> information leakage 0600 would still be prudent.

There is no leakage at all, the permissions are identical to the
permissions on the final registry files. If users don't want their
registry to be readable they can set umask or protect their .wine
directory.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Peter Bortas]

Alexandre Julliard <julliard(a)winehq.org> writes:
> Peter Bortas <peter(a)bortas.org> writes:
> > Home directories are group readable on many sites, so to prevent
> > information leakage 0600 would still be prudent.
>
> There is no leakage at all, the permissions are identical to the
> permissions on the final registry files. If users don't want their
> registry to be readable they can set umask or protect their .wine
> directory.

Excellent.

-- 
Peter Bortas                  http://peter.bortas.org