[PATCH v2 0/2] MR3381: server: Try isolate wineserver directory.

[Konstantin Demin (＠rockdrilla)]

try following locations for wineserver directory:
- `${XDG_RUNTIME_DIR}/wine`
- `/run/user/${uid}/wine`
- `${TMPDIR}/wine` - only if `${TMPDIR}` is owned by user
- `${TMPDIR}/.wine-${uid}`
- `/tmp/.wine-${uid}`

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=39013

Signed-off-by: Konstantin Demin <rockdrilla(a)gmail.com>

--
  v2: server: Avoid wineserver directory race.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3381

[PATCH v2 1/2] server: Try isolate wineserver directory.

[Konstantin Demin]

From: Konstantin Demin <rockdrilla(a)gmail.com>

try following locations for wineserver directory:
- ${XDG_RUNTIME_DIR}/wine
- /run/user/${uid}/wine
- ${TMPDIR}/wine - only if ${TMPDIR} is owned by user
- ${TMPDIR}/.wine-${uid}
- /tmp/.wine-${uid}

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=39013

Signed-off-by: Konstantin Demin <rockdrilla(a)gmail.com>
---
 configure                |   6 ++
 configure.ac             |   1 +
 dlls/ntdll/unix/server.c | 144 ++++++++++++++++++++++++++++-
 include/config.h.in      |   3 +
 server/request.c         | 190 ++++++++++++++++++++++++++++++++++++---
 5 files changed, 326 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index 7e0d8d7621d..789100c1558 100755
--- a/configure
+++ b/configure
@@ -20010,6 +20010,12 @@ if test "x$ac_cv_func_sched_yield" = xyes
 then :
   printf "%s\n" "#define HAVE_SCHED_YIELD 1" >>confdefs.h
 
+fi
+ac_fn_c_check_func "$LINENO" "secure_getenv" "ac_cv_func_secure_getenv"
+if test "x$ac_cv_func_secure_getenv" = xyes
+then :
+  printf "%s\n" "#define HAVE_SECURE_GETENV 1" >>confdefs.h
+
 fi
 ac_fn_c_check_func "$LINENO" "setproctitle" "ac_cv_func_setproctitle"
 if test "x$ac_cv_func_setproctitle" = xyes
diff --git a/configure.ac b/configure.ac
index b32d2ba5aa4..bc1c27a6cff 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2049,6 +2049,7 @@ AC_CHECK_FUNCS(\
 	prctl \
 	proc_pidinfo \
 	sched_yield \
+	secure_getenv \
 	setproctitle \
 	setprogname \
 	sigprocmask \
diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c
index e5e234d05ae..4077544d500 100644
--- a/dlls/ntdll/unix/server.c
+++ b/dlls/ntdll/unix/server.c
@@ -1238,13 +1238,37 @@ int server_pipe( int fd[2] )
 }
 
 
+/***********************************************************************
+ *           try_dir
+ */
+static int try_dir( const char *name, struct stat *st, int per_user )
+{
+    if (lstat( name, st ) == -1) return 1;
+    if (!S_ISDIR( st->st_mode ))
+    {
+        errno = ENOTDIR;
+        return 2;
+    }
+    if (per_user)
+    {
+        if (st->st_uid != getuid()) return 3;
+        if (st->st_mode & 077) return 4;
+    }
+
+    return 0;
+}
+
+
 /***********************************************************************
  *           init_server_dir
  */
 static const char *init_server_dir( dev_t dev, ino_t ino )
 {
-    char *p, *dir;
-    size_t len = sizeof("/server-") + 2 * sizeof(dev) + 2 * sizeof(ino) + 2;
+    char *p, *dir, *server_root_dir;
+    struct stat st;
+    size_t len, len2;
+
+    len = sizeof("/server-") + 2 * sizeof(dev) + 2 * sizeof(ino) + 2;
 
 #ifdef __ANDROID__  /* there's no /tmp dir on Android */
     len += strlen( config_dir ) + sizeof("/.wineserver");
@@ -1252,9 +1276,121 @@ static const char *init_server_dir( dev_t dev, ino_t ino )
     strcpy( dir, config_dir );
     strcat( dir, "/.wineserver/server-" );
 #else
+
+    /*
+     * try following locations for wineserver directory:
+     * - ${XDG_RUNTIME_DIR}/wine
+     * - /run/user/${uid}/wine
+     * - ${TMPDIR}/wine - only if ${TMPDIR} is per-user
+     * - ${TMPDIR}/.wine-${uid}
+     * - /tmp/.wine-${uid}
+     */
+
+    dir = NULL;
+
+    /* try "${XDG_RUNTIME_DIR}/wine" */
+
+#ifdef HAVE_SECURE_GETENV
+    server_root_dir = secure_getenv( "XDG_RUNTIME_DIR" );
+#else
+    server_root_dir = getenv( "XDG_RUNTIME_DIR" );
+#endif
+
+    if (!server_root_dir)
+        goto server_dir_at_run;
+
+    if (try_dir( server_root_dir, &st, 1 ))
+        goto server_dir_at_run;
+
+    len2 = len + strlen(server_root_dir) + sizeof("/wine") + 2;
+    if (!(dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+    sprintf( dir, "%s/wine", server_root_dir );
+    if (!try_dir( dir, &st, 1 ))
+    {
+        len = len2;
+        goto server_dir_done;
+    }
+
+    free( dir );
+    dir = NULL;
+
+server_dir_at_run:
+
+    /* try "/run/user/${uid}/wine" */
+
+    len2 = len + sizeof("/run/user/") + sizeof("/wine") + 12;
+    if (!(dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+    sprintf( dir, "%s%u", "/run/user/", getuid() );
+    if (try_dir( dir, &st, 1 ))
+    {
+        free( dir );
+        dir = NULL;
+        goto server_dir_at_env_tmpdir;
+    }
+
+    strcat( dir, "/wine" );
+    if (!try_dir( dir, &st, 1 ))
+    {
+        len = len2;
+        goto server_dir_done;
+    }
+
+    free( dir );
+    dir = NULL;
+
+server_dir_at_env_tmpdir:
+
+    /* try somewhere in ${TMPDIR}/ */
+
+#ifdef HAVE_SECURE_GETENV
+    server_root_dir = secure_getenv( "TMPDIR" );
+#else
+    server_root_dir = getenv( "TMPDIR" );
+#endif
+
+    if (!server_root_dir)
+        goto server_dir_at_tmp;
+
+    if (try_dir( server_root_dir, &st, 0 ))
+        goto server_dir_at_tmp;
+
+    len2 = len + strlen(server_root_dir);
+    if (try_dir( server_root_dir, &st, 1 ))
+    {
+        /* try "${TMPDIR}/.wine-${uid}" because ${TMPDIR} is not per-user */
+
+        len2 += sizeof("/.wine-") + 12;
+        if (!(dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+        sprintf( dir, "%s/.wine-%u", server_root_dir, getuid() );
+    }
+    else
+    {
+        /* try "${TMPDIR}/wine" because ${TMPDIR} is per-user */
+
+        len2 += sizeof("/wine") + 2;
+        if (!(dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+        sprintf( dir, "%s/wine", server_root_dir );
+    }
+
+    if (!try_dir( dir, &st, 1 ))
+    {
+        len = len2;
+        goto server_dir_done;
+    }
+
+    free( dir );
+    dir = NULL;
+
+server_dir_at_tmp:
+
+    /* try "/tmp/.wine-${uid}" - last resort */
+
     len += sizeof("/tmp/.wine-") + 12;
-    dir = malloc( len );
-    sprintf( dir, "/tmp/.wine-%u/server-", getuid() );
+    if (!(dir = malloc( len ))) fatal_error( "out of memory\n" );
+    sprintf( dir, "/tmp/.wine-%u", getuid() );
+
+server_dir_done:
+    strcat( dir, "/server-" );
 #endif
     p = dir + strlen( dir );
     if (dev != (unsigned long)dev)
diff --git a/include/config.h.in b/include/config.h.in
index 2e806157ebd..43427e152dc 100644
--- a/include/config.h.in
+++ b/include/config.h.in
@@ -366,6 +366,9 @@
 /* Define to 1 if you have the <SDL.h> header file. */
 #undef HAVE_SDL_H
 
+/* Define to 1 if you have the `secure_getenv' function. */
+#undef HAVE_SECURE_GETENV
+
 /* Define to 1 if you have the `setproctitle' function. */
 #undef HAVE_SETPROCTITLE
 
diff --git a/server/request.c b/server/request.c
index 7021741c765..35236292acb 100644
--- a/server/request.c
+++ b/server/request.c
@@ -592,30 +592,79 @@ static void socket_cleanup(void)
     if (!do_it_once++) unlink( server_socket_name );
 }
 
+/* try stat a directory */
+static int try_dir( const char *name, struct stat *st, int per_user )
+{
+    if (lstat( name, st ) == -1) return 1;
+    if (!S_ISDIR( st->st_mode ))
+    {
+        errno = ENOTDIR;
+        return 2;
+    }
+    if (per_user)
+    {
+        if (st->st_uid != getuid()) return 3;
+        if (st->st_mode & 077) return 4;
+    }
+
+    return 0;
+}
+
+/* try create a directory and check its permissions */
+static int try_create_dir( const char *name, struct stat *st )
+{
+    int r = try_dir( name, st, 1 );
+    if (!r) return 0;
+
+    if (r == 1)
+    {
+        r = 0;
+
+        if (errno != ENOENT) return 1;
+        if ((mkdir( name, 0700 ) == -1) && (errno != EEXIST))
+            return 2;
+        if (lstat( name, st ) == -1) return 3;
+
+        if (!S_ISDIR( st->st_mode ))
+            /* passthrough to next block */
+            r = 2;
+    }
+    if (r == 2)
+    {
+        errno = ENOTDIR;
+        return 4;
+    }
+
+    if (st->st_uid != getuid()) return 5;
+    if (st->st_mode & 077) return 6;
+
+    return 0;
+}
+
 /* create a directory and check its permissions */
 static void create_dir( const char *name, struct stat *st )
 {
-    if (lstat( name, st ) == -1)
+    switch( try_create_dir( name, st ) )
     {
-        if (errno != ENOENT)
-            fatal_error( "lstat %s: %s\n", name, strerror( errno ));
-        if (mkdir( name, 0700 ) == -1 && errno != EEXIST)
-            fatal_error( "mkdir %s: %s\n", name, strerror( errno ));
-        if (lstat( name, st ) == -1)
-            fatal_error( "lstat %s: %s\n", name, strerror( errno ));
-    }
-    if (!S_ISDIR(st->st_mode)) fatal_error( "%s is not a directory\n", name );
-    if (st->st_uid != getuid()) fatal_error( "%s is not owned by you\n", name );
-    if (st->st_mode & 077) fatal_error( "%s must not be accessible by other users\n", name );
+    case 0: return;
+    case 1: fatal_error( "lstat %s: %s\n", name, strerror( errno ));
+    case 2: fatal_error( "mkdir %s: %s\n", name, strerror( errno ));
+    case 3: fatal_error( "lstat %s: %s\n", name, strerror( errno ));
+    case 4: fatal_error( "%s is not a directory\n", name );
+    case 5: fatal_error( "%s is not owned by you\n", name );
+    case 6: fatal_error( "%s must not be accessible by other users\n", name );
+    }
 }
 
 /* create the server directory and chdir to it */
 static char *create_server_dir( int force )
 {
     const char *prefix = getenv( "WINEPREFIX" );
-    char *p, *config_dir;
+    char *p, *config_dir, *server_root_dir;
     struct stat st, st2;
-    size_t len = sizeof("/server-") + 2 * sizeof(st.st_dev) + 2 * sizeof(st.st_ino) + 2;
+    size_t len, len2;
+
+    len = sizeof("/server-") + 2 * sizeof(st.st_dev) + 2 * sizeof(st.st_ino) + 2;
 
     /* open the configuration directory */
 
@@ -662,13 +711,126 @@ static char *create_server_dir( int force )
     if (!(server_dir = malloc( len ))) fatal_error( "out of memory\n" );
     strcpy( server_dir, config_dir );
     strcat( server_dir, "/.wineserver" );
+    create_dir( server_dir, &st2 );
+#else
+
+    /*
+     * try following locations for wineserver directory:
+     * - ${XDG_RUNTIME_DIR}/wine
+     * - /run/user/${uid}/wine
+     * - ${TMPDIR}/wine - only if ${TMPDIR} is per-user
+     * - ${TMPDIR}/.wine-${uid}
+     * - /tmp/.wine-${uid}
+     */
+
+    server_dir = NULL;
+
+    /* try "${XDG_RUNTIME_DIR}/wine" */
+
+#ifdef HAVE_SECURE_GETENV
+    server_root_dir = secure_getenv( "XDG_RUNTIME_DIR" );
 #else
+    server_root_dir = getenv( "XDG_RUNTIME_DIR" );
+#endif
+
+    if (!server_root_dir)
+        goto server_dir_at_run;
+
+    if (try_dir( server_root_dir, &st2, 1 ))
+        goto server_dir_at_run;
+
+    len2 = len + strlen(server_root_dir) + sizeof("/wine") + 2;
+    if (!(server_dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+    sprintf( server_dir, "%s/wine", server_root_dir );
+    if (!try_create_dir( server_dir, &st2 ))
+    {
+        len = len2;
+        goto server_dir_done;
+    }
+
+    free( server_dir );
+    server_dir = NULL;
+
+server_dir_at_run:
+
+    /* try "/run/user/${uid}/wine" */
+
+    len2 = len + sizeof("/run/user/") + sizeof("/wine") + 12;
+    if (!(server_dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+    sprintf( server_dir, "%s%u", "/run/user/", getuid() );
+    if (try_dir( server_dir, &st2, 1 ))
+    {
+        free( server_dir );
+        server_dir = NULL;
+        goto server_dir_at_env_tmpdir;
+    }
+
+    strcat( server_dir, "/wine" );
+    if (!try_create_dir( server_dir, &st2 ))
+    {
+        len = len2;
+        goto server_dir_done;
+    }
+
+    free( server_dir );
+    server_dir = NULL;
+
+server_dir_at_env_tmpdir:
+
+    /* try somewhere in ${TMPDIR}/ */
+
+#ifdef HAVE_SECURE_GETENV
+    server_root_dir = secure_getenv( "TMPDIR" );
+#else
+    server_root_dir = getenv( "TMPDIR" );
+#endif
+
+    if (!server_root_dir)
+        goto server_dir_at_tmp;
+
+    if (try_dir( server_root_dir, &st2, 0 ))
+        goto server_dir_at_tmp;
+
+    len2 = len + strlen(server_root_dir);
+    if (try_dir( server_root_dir, &st2, 1 ))
+    {
+        /* try "${TMPDIR}/.wine-${uid}" because ${TMPDIR} is not per-user */
+
+        len2 += sizeof("/.wine-") + 12;
+        if (!(server_dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+        sprintf( server_dir, "%s/.wine-%u", server_root_dir, getuid() );
+    }
+    else
+    {
+        /* try "${TMPDIR}/wine" because ${TMPDIR} is per-user */
+
+        len2 += sizeof("/wine") + 2;
+        if (!(server_dir = malloc( len2 ))) fatal_error( "out of memory\n" );
+        sprintf( server_dir, "%s/wine", server_root_dir );
+    }
+
+    if (!try_create_dir( server_dir, &st2 ))
+    {
+        len = len2;
+        goto server_dir_done;
+    }
+
+    free( server_dir );
+    server_dir = NULL;
+
+server_dir_at_tmp:
+
+    /* try "/tmp/.wine-${uid}" - last resort */
+
     len += sizeof("/tmp/.wine-") + 12;
     if (!(server_dir = malloc( len ))) fatal_error( "out of memory\n" );
     sprintf( server_dir, "/tmp/.wine-%u", getuid() );
-#endif
     create_dir( server_dir, &st2 );
 
+server_dir_done:
+
+#endif
+
     /* now create the server directory */
 
     strcat( server_dir, "/server-" );
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3381

[PATCH v2 2/2] server: Avoid wineserver directory race.

[Konstantin Demin]

From: Konstantin Demin <rockdrilla(a)gmail.com>

Fixes: 5a955934

Signed-off-by: Konstantin Demin <rockdrilla(a)gmail.com>
---
 dlls/ntdll/unix/server.c | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c
index 4077544d500..a521d0543d5 100644
--- a/dlls/ntdll/unix/server.c
+++ b/dlls/ntdll/unix/server.c
@@ -1305,14 +1305,8 @@ static const char *init_server_dir( dev_t dev, ino_t ino )
     len2 = len + strlen(server_root_dir) + sizeof("/wine") + 2;
     if (!(dir = malloc( len2 ))) fatal_error( "out of memory\n" );
     sprintf( dir, "%s/wine", server_root_dir );
-    if (!try_dir( dir, &st, 1 ))
-    {
-        len = len2;
-        goto server_dir_done;
-    }
-
-    free( dir );
-    dir = NULL;
+    len = len2;
+    goto server_dir_done;
 
 server_dir_at_run:
 
@@ -1329,14 +1323,8 @@ server_dir_at_run:
     }
 
     strcat( dir, "/wine" );
-    if (!try_dir( dir, &st, 1 ))
-    {
-        len = len2;
-        goto server_dir_done;
-    }
-
-    free( dir );
-    dir = NULL;
+    len = len2;
+    goto server_dir_done;
 
 server_dir_at_env_tmpdir:
 
@@ -1372,14 +1360,8 @@ server_dir_at_env_tmpdir:
         sprintf( dir, "%s/wine", server_root_dir );
     }
 
-    if (!try_dir( dir, &st, 1 ))
-    {
-        len = len2;
-        goto server_dir_done;
-    }
-
-    free( dir );
-    dir = NULL;
+    len = len2;
+    goto server_dir_done;
 
 server_dir_at_tmp:
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3381