sesman: env_set_user, fix potential bof issuesv0.8

Conflicts:
	common/os_calls.h

5 files changed, 85 insertions, 38 deletions

diff --git a/common/os_calls.c b/common/os_calls.c
index bfea9031..95250680 100644
--- a/common/os_calls.c
+++ b/common/os_calls.c
@@ -253,14 +253,17 @@ g_sprintf(char *dest, const char *format, ...)
 }
 
 /*****************************************************************************/
-void DEFAULT_CC
+int DEFAULT_CC
 g_snprintf(char *dest, int len, const char *format, ...)
 {
+    int err;
     va_list ap;
 
     va_start(ap, format);
-    vsnprintf(dest, len, format, ap);
+    err = vsnprintf(dest, len, format, ap);
     va_end(ap);
+
+    return err;
 }
 
 /*****************************************************************************/
@@ -3017,10 +3020,11 @@ g_sigterm(int pid)
 
 /*****************************************************************************/
 /* returns 0 if ok */
+/* the caller is responsible to free the buffs */
 /* does not work in win32 */
 int APP_CC
-g_getuser_info(const char *username, int *gid, int *uid, char *shell,
-               char *dir, char *gecos)
+g_getuser_info(const char *username, int *gid, int *uid, char **shell,
+               char **dir, char **gecos)
 {
 #if defined(_WIN32)
     return 1;
@@ -3043,17 +3047,17 @@ g_getuser_info(const char *username, int *gid, int *uid, char *shell,
 
         if (dir != 0)
         {
-            g_strcpy(dir, pwd_1->pw_dir);
+            *dir = g_strdup(pwd_1->pw_dir);
         }
 
         if (shell != 0)
         {
-            g_strcpy(shell, pwd_1->pw_shell);
+            *shell = g_strdup(pwd_1->pw_shell);
         }
 
         if (gecos != 0)
         {
-            g_strcpy(gecos, pwd_1->pw_gecos);
+            *gecos = g_strdup(pwd_1->pw_gecos);
         }
 
         return 0;
diff --git a/common/os_calls.h b/common/os_calls.h
index d954a075..251896b0 100644
--- a/common/os_calls.h
+++ b/common/os_calls.h
@@ -35,7 +35,7 @@ void* APP_CC    g_malloc(int size, int zero);
 void APP_CC     g_free(void* ptr);
 void DEFAULT_CC g_printf(const char *format, ...);
 void DEFAULT_CC g_sprintf(char* dest, const char* format, ...);
-void DEFAULT_CC g_snprintf(char* dest, int len, const char* format, ...);
+int  DEFAULT_CC g_snprintf(char* dest, int len, const char* format, ...);
 void DEFAULT_CC g_writeln(const char* format, ...);
 void DEFAULT_CC g_write(const char* format, ...);
 void APP_CC     g_hexdump(char* p, int len);
@@ -154,8 +154,8 @@ char* APP_CC    g_getenv(const char* name);
 int APP_CC      g_exit(int exit_code);
 int APP_CC      g_getpid(void);
 int APP_CC      g_sigterm(int pid);
-int APP_CC      g_getuser_info(const char* username, int* gid, int* uid, char* shell,
-                               char* dir, char* gecos);
+int APP_CC      g_getuser_info(const char* username, int* gid, int* uid, char** shell,
+                               char** dir, char** gecos);
 int APP_CC      g_getgroup_info(const char* groupname, int* gid);
 int APP_CC      g_check_user_in_group(const char* username, int gid, int* ok);
 int APP_CC      g_time1(void);
diff --git a/sesman/env.c b/sesman/env.c
index 227f6bbf..b6fa22c3 100644
--- a/sesman/env.c
+++ b/sesman/env.c
@@ -61,8 +61,9 @@ env_check_password_file(char *filename, char *password)
 }
 
 /******************************************************************************/
+/*  its the responsibility of the caller to free passwd_file                  */
 int DEFAULT_CC
-env_set_user(char *username, char *passwd_file, int display,
+env_set_user(char *username, char **passwd_file, int display,
              struct list *env_names, struct list* env_values)
 {
     int error;
@@ -70,15 +71,17 @@ env_set_user(char *username, char *passwd_file, int display,
     int pw_gid;
     int uid;
     int index;
+    int len;
     char *name;
     char *value;
-    char pw_shell[256];
-    char pw_dir[256];
-    char pw_gecos[256];
+    char *pw_shell;
+    char *pw_dir;
     char text[256];
 
-    error = g_getuser_info(username, &pw_gid, &pw_uid, pw_shell, pw_dir,
-                           pw_gecos);
+    pw_shell = 0;
+    pw_dir = 0;
+
+    error = g_getuser_info(username, &pw_gid, &pw_uid, &pw_shell, &pw_dir, 0);
 
     if (error == 0)
     {
@@ -128,28 +131,48 @@ env_set_user(char *username, char *passwd_file, int display,
                 if (0 == g_cfg->auth_file_path)
                 {
                     /* if no auth_file_path is set, then we go for
-                       $HOME/.vnc/sesman_username_passwd */
+                     $HOME/.vnc/sesman_username_passwd */
                     if (g_mkdir(".vnc") < 0)
                     {
                         log_message(LOG_LEVEL_ERROR,
-                            "env_set_user: error creating .vnc dir");
+                                    "env_set_user: error creating .vnc dir");
+                    }
+
+                    len = g_snprintf(NULL, 0, "%s/.vnc/sesman_%s_passwd", pw_dir, username);
+
+                    *passwd_file = (char *) g_malloc(len + 1, 1);
+                    if (*passwd_file != NULL)
+                    {
+                        g_sprintf(*passwd_file, "%s/.vnc/sesman_%s_passwd", pw_dir, username);
                     }
-                    g_sprintf(passwd_file, "%s/.vnc/sesman_%s_passwd", pw_dir, username);
                 }
                 else
                 {
                     /* we use auth_file_path as requested */
-                    g_sprintf(passwd_file, g_cfg->auth_file_path, username);
+                    len = g_snprintf(NULL, 0, g_cfg->auth_file_path, username);
+
+                    *passwd_file = (char *) g_malloc(len + 1, 1);
+                    if (*passwd_file != NULL)
+                    {
+                        g_sprintf(*passwd_file, g_cfg->auth_file_path, username);
+                    }
                 }
 
-                LOG_DBG("pass file: %s", passwd_file);
+                if (*passwd_file != NULL)
+                {
+                    LOG_DBG("pass file: %s", *passwd_file);
+                }
             }
+
+            g_free(pw_dir);
+            g_free(pw_shell);
         }
     }
     else
     {
         log_message(LOG_LEVEL_ERROR,
-                    "error getting user info for user %s", username);
+                    "error getting user info for user %s",
+                    username);
     }
 
     return error;
diff --git a/sesman/env.h b/sesman/env.h
index 50473a01..a5f337d1 100644
--- a/sesman/env.h
+++ b/sesman/env.h
@@ -50,7 +50,7 @@ env_check_password_file(char* filename, char* password);
  *
  */
 int DEFAULT_CC
-env_set_user(char* username, char* passwd_file, int display,
+env_set_user(char* username, char** passwd_file, int display,
              struct list *env_names, struct list* env_values);
 
 #endif
diff --git a/sesman/session.c b/sesman/session.c
index 4ea48d35..d4cbcdc1 100644
--- a/sesman/session.c
+++ b/sesman/session.c
@@ -324,8 +324,11 @@ session_start_sessvc(int xpid, int wmpid, long data, char *username, int display
     list_add_item(sessvc_params, (long)g_strdup(wmpid_str));
     list_add_item(sessvc_params, 0); /* mandatory */
 
-    env_set_user(username, 0, display,
-                 g_cfg->session_variables1, g_cfg->session_variables2);
+    env_set_user(username,
+                 0,
+                 display,
+                 g_cfg->session_variables1,
+                 g_cfg->session_variables2);
 
     /* executing sessvc */
     g_execvp(exe_path, ((char **)sessvc_params->items));
@@ -459,14 +462,14 @@ session_start_fork(int width, int height, int bpp, char *username,
     char depth[32];
     char screen[32];
     char text[256];
-    char passwd_file[256];
-    char *pfile;
+    char execvpparams[2048];
+    char *xserver; /* absolute/relative path to Xorg/X11rdp/Xvnc */
+    char *passwd_file;
     char **pp1 = (char **)NULL;
     struct session_chain *temp = (struct session_chain *)NULL;
     struct list *xserver_params = (struct list *)NULL;
-    time_t ltime;
     struct tm stime;
-    char execvpparams[2048];
+    time_t ltime;
 
     /* initialize (zero out) local variables: */
     g_memset(&ltime, 0, sizeof(time_t));
@@ -475,7 +478,8 @@ session_start_fork(int width, int height, int bpp, char *username,
     g_memset(depth, 0, sizeof(char) * 32);
     g_memset(screen, 0, sizeof(char) * 32);
     g_memset(text, 0, sizeof(char) * 256);
-    g_memset(passwd_file, 0, sizeof(char) * 256);
+
+    passwd_file = 0;
 
     /* check to limit concurrent sessions */
     if (g_session_count >= g_cfg->sess.max_sessions)
@@ -539,7 +543,9 @@ session_start_fork(int width, int height, int bpp, char *username,
             }
             else if (pampid == 0) /* child: X11/client */
             {
-                env_set_user(username, 0, display,
+                env_set_user(username,
+                             0,
+                             display,
                              g_cfg->session_variables1,
                              g_cfg->session_variables2);
                 if (x_server_running(display))
@@ -634,14 +640,23 @@ session_start_fork(int width, int height, int bpp, char *username,
             }
             else if (xpid == 0) /* child */
             {
-                pfile = 0;
                 if (type == SESMAN_SESSION_TYPE_XVNC)
                 {
-                    pfile = passwd_file;
+                    env_set_user(username,
+                                 &passwd_file,
+                                 display,
+                                 g_cfg->session_variables1,
+                                 g_cfg->session_variables2);
                 }
-                env_set_user(username, pfile, display,
-                             g_cfg->session_variables1,
-                             g_cfg->session_variables2);
+                else
+                {
+                    env_set_user(username,
+                                 0,
+                                 display,
+                                 g_cfg->session_variables1,
+                                 g_cfg->session_variables2);
+                }
+
 
                 g_snprintf(text, 255, "%d", g_cfg->sess.max_idle_time);
                 g_setenv("XRDP_SESMAN_MAX_IDLE_TIME", text, 1);
@@ -695,6 +710,8 @@ session_start_fork(int width, int height, int bpp, char *username,
                     list_add_item(xserver_params, (long)g_strdup("-rfbauth"));
                     list_add_item(xserver_params, (long)g_strdup(passwd_file));
 
+                    g_free(passwd_file);
+
                     /* additional parameters from sesman.ini file */
                     //config_read_xserver_params(SESMAN_SESSION_TYPE_XVNC,
                     //                           xserver_params);
@@ -825,8 +842,11 @@ session_reconnect_fork(int display, char *username)
     }
     else if (pid == 0)
     {
-        env_set_user(username, 0, display,
-                     g_cfg->session_variables1, g_cfg->session_variables2);
+        env_set_user(username,
+                     0,
+                     display,
+                     g_cfg->session_variables1,
+                     g_cfg->session_variables2);
         g_snprintf(text, 255, "%s/%s", XRDP_CFG_PATH, "reconnectwm.sh");
 
         if (g_file_exist(text))