[PATCH] string_to_sid audit

[Martin Pool]

In several cases, the return code from string_to_sid is not checked.
So if the user enters a syntactically invalid SID, the program will
proceed to use uninitialized data.

This patch checks for a few such cases that I found.  Can somebody
please review it?

Index: groupdb/mapping.c
===================================================================
RCS file: /data/cvs/samba/source/groupdb/mapping.c,v
retrieving revision 1.44
diff -u -U6 -r1.44 mapping.c
--- groupdb/mapping.c	2 Jan 2003 09:07:02 -0000	1.44
+++ groupdb/mapping.c	17 Feb 2003 04:21:37 -0000
@@ -301,13 +301,17 @@
 	if(!init_group_mapping()) {
 		DEBUG(0,("failed to initialize group mapping"));
 		return(False);
 	}
 	
 	map.gid=gid;
-	string_to_sid(&map.sid, sid);
+	if (!string_to_sid(&map.sid, sid)) {
+		DEBUG(0, ("string_to_sid failed: %s", sid));
+		return False;
+	}
+	
 	map.sid_name_use=sid_name_use;
 	fstrcpy(map.nt_name, nt_name);
 	fstrcpy(map.comment, comment);
 	map.systemaccount=systemaccount;
 
 	map.priv_set.count=priv_set.count;
Index: nsswitch/wb_client.c
===================================================================
RCS file: /data/cvs/samba/source/nsswitch/wb_client.c,v
retrieving revision 1.40
diff -u -U6 -r1.40 wb_client.c
--- nsswitch/wb_client.c	7 Aug 2002 07:28:24 -0000	1.40
+++ nsswitch/wb_client.c	17 Feb 2003 04:21:37 -0000
@@ -53,13 +53,14 @@
 
 	fstrcpy(request.data.name.dom_name, dom_name);
 	fstrcpy(request.data.name.name, name);
 
 	if ((result = winbindd_request(WINBINDD_LOOKUPNAME, &request, 
 				       &response)) == NSS_STATUS_SUCCESS) {
-		string_to_sid(sid, response.data.sid.sid);
+		if (!string_to_sid(sid, response.data.sid.sid))
+			return False;
 		*name_type = (enum SID_NAME_USE)response.data.sid.type;
 	}
 
 	return result == NSS_STATUS_SUCCESS;
 }
 
@@ -155,13 +156,14 @@
 
 	result = winbindd_request(WINBINDD_UID_TO_SID, &request, &response);
 
 	/* Copy out result */
 
 	if (result == NSS_STATUS_SUCCESS) {
-		string_to_sid(sid, response.data.sid.sid);
+		if (!string_to_sid(sid, response.data.sid.sid))
+			return False;
 	} else {
 		sid_copy(sid, &global_sid_NULL);
 	}
 
 	return (result == NSS_STATUS_SUCCESS);
 }
@@ -221,13 +223,14 @@
 
 	result = winbindd_request(WINBINDD_GID_TO_SID, &request, &response);
 
 	/* Copy out result */
 
 	if (result == NSS_STATUS_SUCCESS) {
-		string_to_sid(sid, response.data.sid.sid);
+		if (!string_to_sid(sid, response.data.sid.sid))
+			return False;
 	} else {
 		sid_copy(sid, &global_sid_NULL);
 	}
 
 	return (result == NSS_STATUS_SUCCESS);
 }
Index: python/py_lsa.c
===================================================================
RCS file: /data/cvs/samba/source/python/py_lsa.c,v
retrieving revision 1.16
diff -u -U6 -r1.16 py_lsa.c
--- python/py_lsa.c	23 Dec 2002 23:53:55 -0000	1.16
+++ python/py_lsa.c	17 Feb 2003 04:21:37 -0000
@@ -232,23 +232,29 @@
 		
 		memset(sids, 0, num_sids * sizeof(DOM_SID));
 		
 		for (i = 0; i < num_sids; i++) {
 			PyObject *obj = PyList_GetItem(py_sids, i);
 			
-			string_to_sid(&sids[i], PyString_AsString(obj));
+			if (!string_to_sid(&sids[i], PyString_AsString(obj))) {
+				PyErr_SetString(PyExc_ValueError, "string_to_sid failed");
+				return NULL;
+			}
 		}
 
 	} else {
 
 		/* Just a single element */
 
 		num_sids = 1;
 		sids = (DOM_SID *)talloc(hnd->mem_ctx, sizeof(DOM_SID));
 
-		string_to_sid(&sids[0], PyString_AsString(py_sids));
+		if (!string_to_sid(&sids[0], PyString_AsString(py_sids))) {
+			PyErr_SetString(PyExc_ValueError, "string_to_sid failed");
+			return NULL;
+		}
 	}
 
 	ntstatus = cli_lsa_lookup_sids(hnd->cli, hnd->mem_ctx, &hnd->pol,
 				       num_sids, sids, &domains, &names, 
 				       &types);
 
Index: rpc_parse/parse_net.c
===================================================================
RCS file: /data/cvs/samba/source/rpc_parse/parse_net.c,v
retrieving revision 1.102
diff -u -U6 -r1.102 parse_net.c
--- rpc_parse/parse_net.c	14 Feb 2003 23:04:02 -0000	1.102
+++ rpc_parse/parse_net.c	17 Feb 2003 04:21:38 -0000
@@ -882,17 +882,19 @@
 		*ppsids = (DOM_SID2 *)talloc_zero(ctx, count * sizeof(DOM_SID2));
 		if (*ppsids == NULL)
 			return 0;
 
 		sids = *ppsids;
 
-		for (number = 0, ptr = sids_str; 
-	   	  next_token(&ptr, s2, NULL, sizeof(s2)); number++) {
+		for (number = 0, ptr = sids_str; next_token(&ptr, s2, NULL, sizeof(s2)); ) {
 			DOM_SID tmpsid;
-			string_to_sid(&tmpsid, s2);
-			init_dom_sid2(&sids[number], &tmpsid);
+			if (string_to_sid(&tmpsid, s2)) {
+				/* count only valid sids */
+				init_dom_sid2(&sids[number], &tmpsid);
+				number++;
+			}
 		}
 	}
 
 	return count;
 }
 
Index: rpcclient/cmd_lsarpc.c
===================================================================
RCS file: /data/cvs/samba/source/rpcclient/cmd_lsarpc.c,v
retrieving revision 1.72
diff -u -U6 -r1.72 cmd_lsarpc.c
--- rpcclient/cmd_lsarpc.c	10 Feb 2003 11:31:23 -0000	1.72
+++ rpcclient/cmd_lsarpc.c	17 Feb 2003 04:21:38 -0000
@@ -207,14 +207,17 @@
 
 	if (!sids) {
 		printf("could not allocate memory for %d sids\n", argc - 1);
 		goto done;
 	}
 
-	for (i = 0; i < argc - 1; i++)
-		string_to_sid(&sids[i], argv[i + 1]);
+	for (i = 0; i < argc - 1; i++) 
+		if (!string_to_sid(&sids[i], argv[i + 1])) {
+			result = NT_STATUS_INVALID_SID;
+			goto done;
+		}
 
 	/* Lookup the SIDs */
 
 	result = cli_lsa_lookup_sids(cli, mem_ctx, &pol, argc - 1, sids, 
 				     &domains, &names, &types);
 


-- 
Martin