Bug 5198 - Gecos field is passed as full name

[Jesse Malone]

Well, no luck so far. From what I can tell samu_set_unix_internal()
doesn't get called when a user logs on to the domain. I put some debugs in
both samu_set_unix_internal() and count_commas() but only see them logged
in log.smbd when I launch smbd, not when a user logs in. In
log.*machine_name* I only see debug messages from pdb_set_fullname(). I
might of course be interpreting event incorrectly. On a hunch I've added
the check for commas to  init_samu_from_buffer_v0() , v1() .... (see the
patch below, against 3.4.3). I haven't had a chance to test this yet.
Thoughts are welcome.

--- ./samba-3.4.3/source3/passdb/passdb.c	2009-10-29 03:47:16.000000000 -0400
+++ ./samba-3.4.3/source3/passdb/passdb.c	2010-02-08 11:12:18.000000000 -0500
@@ -121,6 +121,47 @@
 	return user;
 }

+static int count_commas(const char *str)
+{
+	int num_commas = 0;
+	const char *comma = str;
+
+	while ((comma = strchr(str, ',')) != NULL) {
+		comma += 1;
+		num_commas += 1;
+	}
+
+	DEBUG(10,("count_commas: checking for gecos fields, counted %d
commas",num_commas));
+
+	return num_commas;
+}
+
+/*******************************************
+ Check if fullname is actually a comma seperated
+ GECOS field, and return first GECOS field (full name),
+ otherwise do nothing
+*******************************************/
+char *pdb_check_gecos(char *fullname)
+{
+  char* real_fullname;
+  real_fullname = fullname;
+
+  if (count_commas(fullname) == 3) {
+    /*
+     * Heuristic: This seems to be a gecos field that has been
+     * edited by chfn(1). Only use the part before the first
+     * comma. Fixes bug 5198.
+     */
+    real_fullname = talloc_strndup(
+			      talloc_tos(), fullname,
+			      strchr(fullname, ',') - fullname);
+
+    DEBUG(10,("pdb_check_gecos: found gecos fields, grabbing first
parameter '%s' as fullname",fullname));
+  }
+
+  return real_fullname;
+}
+
 /*********************************************************************
  Initialize a struct samu from a struct passwd including the user
  and group SIDs.  The *user structure is filled out with the Unix
@@ -131,6 +172,7 @@
 {
 	const char *guest_account = lp_guestaccount();
 	const char *domain = global_myname();
+	char *fullname;
 	uint32 urid;

 	if ( !pwd ) {
@@ -140,7 +182,14 @@
 	/* Basic properties based upon the Unix account information */

 	pdb_set_username(user, pwd->pw_name, PDB_SET);
-	pdb_set_fullname(user, pwd->pw_gecos, PDB_SET);
+
+
+	/* get full name from gecos field */
+	fullname = pdb_check_gecos(pwd->pw_gecos);
+	pdb_set_fullname(user,fullname, PDB_SET);
+
+	TALLOC_FREE(fullname);
+
 	pdb_set_domain (user, get_global_sam_name(), PDB_DEFAULT);
 #if 0
 	/* This can lead to a primary group of S-1-22-2-XX which
@@ -914,6 +963,7 @@
 	char *unknown_str = NULL;
 	char *munged_dial = NULL;
 	char *fullname = NULL;
+	char *real_fullname = NULL;
 	char *homedir = NULL;
 	char *logon_script = NULL;
 	char *profile_path = NULL;
@@ -988,7 +1038,13 @@
 	pdb_set_username(sampass, username, PDB_SET);
 	pdb_set_domain(sampass, domain, PDB_SET);
 	pdb_set_nt_username(sampass, nt_username, PDB_SET);
-	pdb_set_fullname(sampass, fullname, PDB_SET);
+
+	/*
+	  fullname may contain comma seperated GECOS fields.
+	  Extract fullname from GECOS
+	*/
+	real_fullname = pdb_check_gecos(fullname);
+	pdb_set_fullname(sampass,real_fullname , PDB_SET);

 	if (homedir) {
 		pdb_set_homedir(sampass, homedir, PDB_SET);
@@ -1100,6 +1156,7 @@
 	char *unknown_str = NULL;
 	char *munged_dial = NULL;
 	char *fullname = NULL;
+	char *real_fullname = NULL;
 	char *homedir = NULL;
 	char *logon_script = NULL;
 	char *profile_path = NULL;
@@ -1179,7 +1236,13 @@
 	pdb_set_username(sampass, username, PDB_SET);
 	pdb_set_domain(sampass, domain, PDB_SET);
 	pdb_set_nt_username(sampass, nt_username, PDB_SET);
-	pdb_set_fullname(sampass, fullname, PDB_SET);
+
+	/*
+	  fullname may contain comma seperated GECOS fields.
+	  Extract fullname from GECOS
+	*/
+	real_fullname = pdb_check_gecos(fullname);
+	pdb_set_fullname(sampass,real_fullname , PDB_SET);

 	if (homedir) {
 		pdb_set_homedir(sampass, homedir, PDB_SET);
@@ -1289,6 +1352,7 @@
 	char *unknown_str = NULL;
 	char *munged_dial = NULL;
 	char *fullname = NULL;
+	char *real_fullname = NULL;
 	char *homedir = NULL;
 	char *logon_script = NULL;
 	char *profile_path = NULL;
@@ -1370,7 +1434,13 @@
 	pdb_set_username(sampass, username, PDB_SET);
 	pdb_set_domain(sampass, domain, PDB_SET);
 	pdb_set_nt_username(sampass, nt_username, PDB_SET);
-	pdb_set_fullname(sampass, fullname, PDB_SET);
+
+	/*
+	  fullname may contain comma seperated GECOS fields.
+	  Extract fullname from GECOS
+	*/
+	real_fullname = pdb_check_gecos(fullname);
+	pdb_set_fullname(sampass,real_fullname , PDB_SET);

 	if (homedir) {
 		fstrcpy( tmp_string, homedir );
@@ -1524,6 +1594,7 @@
 	char *unknown_str = NULL;
 	char *munged_dial = NULL;
 	char *fullname = NULL;
+	char *real_fullname = NULL;
 	char *homedir = NULL;
 	char *logon_script = NULL;
 	char *profile_path = NULL;
@@ -1606,7 +1677,13 @@
 	pdb_set_username(sampass, username, PDB_SET);
 	pdb_set_domain(sampass, domain, PDB_SET);
 	pdb_set_nt_username(sampass, nt_username, PDB_SET);
-	pdb_set_fullname(sampass, fullname, PDB_SET);
+
+	/*
+	  fullname may contain comma seperated GECOS fields.
+	  Extract fullname from GECOS
+	*/
+	real_fullname = pdb_check_gecos(fullname);
+	pdb_set_fullname(sampass,real_fullname , PDB_SET);

 	if (homedir) {
 		fstrcpy( tmp_string, homedir );

------------------------------------------------------------

Jesse



AUTHOR wrote Volker Lendecke
> On Sat, Feb 06, 2010 at 11:33:36AM -0500, simo wrote:
>>
>> Ah nice,
>> your patch looks good.
>>
>> if you set the review flag I'll ack it.
>>
>
> I haven't even run it once....
>
> I wanted to wait for the bug reporter's okay, that's why I
> did not push it anywhere yet.
>
> Volker
>
>