Complete.Org: Mailing Lists: Archives: freeciv-dev: March 2004:
[Freeciv-Dev] (PR#8385) user_username is insane
Home

[Freeciv-Dev] (PR#8385) user_username is insane

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients: ;
Subject: [Freeciv-Dev] (PR#8385) user_username is insane
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 25 Mar 2004 11:09:10 -0800
Reply-to: rt@xxxxxxxxxxx

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8385 >

The user_username function can return names that are not sane.  This 
isn't a fatal problem, but will cause the player to get an unusable 
player name in the conndlg when a better name is surely available.  It 
can also give usernames that are too long.

As an example, try:

   USER="" civclient

This patch fixes it.  The username is truncated when it's copied into 
the username struct, and then tested for sanity.  Note that under some 
charsets (UTF-8) the truncation can turn a sane name into an insane one.

This is still not fully charset-safe, however: the functions 
user_username calls will probably give the username in the local 
charset, whereas the caller of user_username probably wants the name to 
be in the Freeciv internal & network charset (UTF-8).  Nothing can be 
done about it now, though.

I also changed the function to return a "const" char*.  All the callers 
were fine with this.

jason

? Womoks
Index: common/shared.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/shared.c,v
retrieving revision 1.107
diff -u -r1.107 shared.c
--- common/shared.c     25 Mar 2004 18:31:19 -0000      1.107
+++ common/shared.c     25 Mar 2004 19:01:04 -0000
@@ -710,59 +710,73 @@
   Gets value once, and then caches result.
   Note the caller should not mess with returned string.
 ***************************************************************************/
-char *user_username(void)
+const char *user_username(void)
 {
-  static char *username = NULL;          /* allocated once, never free()d */
+  static char username[MAX_LEN_NAME];
 
-  if (username)
+  /* This function uses a number of different methods to try to find a
+   * username.  This username then has to be truncated to MAX_LEN_NAME
+   * characters (including terminator) and checked for sanity.  Note that
+   * truncating a sane name can leave you with an insane name under some
+   * charsets. */
+
+  if (username[0] != '\0') {
+    /* Username is already known; just return it. */
     return username;
+  }
 
+  /* If the environment variable $USER is present and sane, use it. */
   {
     char *env = getenv("USER");
+
     if (env) {
-      username = mystrdup(env);
-      freelog(LOG_VERBOSE, "USER username is %s", username);
-      return username;
+      sz_strlcpy(username, env);
+      if (is_sane_name(username)) {
+       freelog(LOG_VERBOSE, "USER username is %s", username);
+       return username;
+      }
     }
   }
+
 #ifdef HAVE_GETPWUID
+  /* Otherwise if getpwuid() is available we can use it to find the true
+   * username. */
   {
     struct passwd *pwent = getpwuid(getuid());
+
     if (pwent) {
-      username = mystrdup(pwent->pw_name);
-      freelog(LOG_VERBOSE, "getpwuid username is %s", username);
-      return username;
+      sz_strlcpy(username, pwent->pw_name);
+      if (is_sane_name(username)) {
+       freelog(LOG_VERBOSE, "getpwuid username is %s", username);
+       return username;
+      }
     }
   }
 #endif
 
 #ifdef WIN32_NATIVE
+  /* On win32 the GetUserName function will give us the login name. */
   {
     DWORD length;
+    char name[UNLEN + 1];
 
-    /* On Win32 use the GetUserName function to find a name. */
-    username = fc_malloc(UNLEN + 1);
-    if (GetUserName(username, &length)) {
-      /* Length includes the NUL terminator. */
-      if (length > MAX_LEN_NAME) {
-       username[MAX_LEN_NAME - 1] = '\0';
-      }
+    if (GetUserName(name, &length)) {
+      sz_strlcpy(username, name);
       if (is_sane_name(username)) {
        freelog(LOG_VERBOSE, "GetUserName username is %s", username);
        return username;
       }
     }
-    free(username);
   }
 #endif
-  username = fc_malloc(MAX_LEN_NAME);
 
 #ifdef ALWAYS_ROOT
-  my_snprintf(username, MAX_LEN_NAME, "name");
+  sz_strlcpy(username, "name");
 #else
   my_snprintf(username, MAX_LEN_NAME, "name%d", (int)getuid());
 #endif
   freelog(LOG_VERBOSE, "fake username is %s", username);
+  assert(is_sane_name(username));
   return username;
 }
 
Index: common/shared.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/shared.h,v
retrieving revision 1.122
diff -u -r1.122 shared.h
--- common/shared.h     11 Jan 2004 17:45:04 -0000      1.122
+++ common/shared.h     25 Mar 2004 19:01:04 -0000
@@ -197,7 +197,7 @@
       fc__attribute((format (printf, 3, 4)));
 
 char *user_home_dir(void);
-char *user_username(void);
+const char *user_username(void);
 const char **datafilelist(const char *suffix);
 char *datafilename(const char *filename);
 char *datafilename_required(const char *filename);

[Prev in Thread] Current Thread [Next in Thread]
  • [Freeciv-Dev] (PR#8385) user_username is insane, Jason Short <=