winbindd: Use wrapper for string to integer conversion

In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.

Signed-off-by: Swen Schillig <swen@linux.ibm.com>
Reviewed-by: Ralph Böhme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

3 files changed, 52 insertions, 15 deletions

diff --git a/source3/winbindd/idmap_ldap.c b/source3/winbindd/idmap_ldap.c
index 17cc7404f12..822abb9f559 100644
--- a/source3/winbindd/idmap_ldap.c
+++ b/source3/winbindd/idmap_ldap.c
@@ -222,6 +222,7 @@ static NTSTATUS idmap_ldap_allocate_id_internal(struct idmap_domain *dom,
 	const char **attr_list;
 	const char *type;
 	struct idmap_ldap_context *ctx;
+	int error = 0;
 
 	/* Only do query if we are online */
 	if (idmap_is_offline())	{
@@ -299,7 +300,11 @@ static NTSTATUS idmap_ldap_allocate_id_internal(struct idmap_domain *dom,
 		goto done;
 	}
 
-	xid->id = strtoul(id_str, NULL, 10);
+	xid->id = strtoul_err(id_str, NULL, 10, &error);
+	if (error != 0) {
+		ret = NT_STATUS_UNSUCCESSFUL;
+		goto done;
+	}
 
 	/* make sure we still have room to grow */
 
@@ -641,6 +646,7 @@ static NTSTATUS idmap_ldap_unixids_to_sids(struct idmap_domain *dom,
 	int count;
 	int rc;
 	int i;
+	int error = 0;
 
 	/* Only do query if we are online */
 	if (idmap_is_offline())	{
@@ -769,16 +775,23 @@ again:
 			continue;
 		}
 
-		id = strtoul(tmp, NULL, 10);
+		id = strtoul_err(tmp, NULL, 10, &error);
+		TALLOC_FREE(tmp);
+		if (error != 0) {
+			DEBUG(5, ("Requested id (%u) out of range (%u - %u). "
+				  "Filtered!\n", id,
+				  dom->low_id, dom->high_id));
+			TALLOC_FREE(sidstr);
+			continue;
+		}
+
 		if (!idmap_unix_id_is_in_range(id, dom)) {
 			DEBUG(5, ("Requested id (%u) out of range (%u - %u). "
 				  "Filtered!\n", id,
 				  dom->low_id, dom->high_id));
 			TALLOC_FREE(sidstr);
-			TALLOC_FREE(tmp);
 			continue;
 		}
-		TALLOC_FREE(tmp);
 
 		map = idmap_find_map_by_id(&ids[bidx], type, id);
 		if (!map) {
@@ -947,6 +960,7 @@ again:
 		struct dom_sid sid;
 		struct dom_sid_buf buf;
 		uint32_t id;
+		int error = 0;
 
 		if (i == 0) { /* first entry */
 			entry = ldap_first_entry(
@@ -1005,16 +1019,23 @@ again:
 			continue;
 		}
 
-		id = strtoul(tmp, NULL, 10);
-		if (!idmap_unix_id_is_in_range(id, dom)) {
+		id = strtoul_err(tmp, NULL, 10, &error);
+		TALLOC_FREE(tmp);
+		if (error != 0) {
+			DEBUG(5, ("Requested id (%u) out of range (%u - %u). "
+				  "Filtered!\n", id,
+				  dom->low_id, dom->high_id));
+			TALLOC_FREE(sidstr);
+			continue;
+		}
+
+		if (error != 0 || !idmap_unix_id_is_in_range(id, dom)) {
 			DEBUG(5, ("Requested id (%u) out of range (%u - %u). "
 				  "Filtered!\n", id,
 				  dom->low_id, dom->high_id));
 			TALLOC_FREE(sidstr);
-			TALLOC_FREE(tmp);
 			continue;
 		}
-		TALLOC_FREE(tmp);
 
 		if (map->status == ID_MAPPED) {
 			DEBUG(1, ("WARNING: duplicate %s mapping in LDAP. "
diff --git a/source3/winbindd/winbindd_lookuprids.c b/source3/winbindd/winbindd_lookuprids.c
index 1e80b78a92e..959a4a7db38 100644
--- a/source3/winbindd/winbindd_lookuprids.c
+++ b/source3/winbindd/winbindd_lookuprids.c
@@ -182,8 +182,10 @@ static bool parse_ridlist(TALLOC_CTX *mem_ctx, char *ridstr,
 
 	for (i=0; i<num_rids; i++) {
 		char *q;
-		rids[i] = strtoul(p, &q, 10);
-		if (*q != '\n') {
+		int error = 0;
+
+		rids[i] = strtoul_err(p, &q, 10, &error);
+		if (error != 0 || *q != '\n') {
 			DEBUG(0, ("Got invalid ridstr: %s\n", p));
 			return false;
 		}
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index d266eb3048e..91a2f6ef197 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -461,6 +461,7 @@ static void trustdom_list_done(struct tevent_req *req)
 		uint32_t trust_type;
 		uint32_t trust_attribs;
 		uint32_t trust_flags;
+		int error = 0;
 
 		DBG_DEBUG("parsing response line '%s'\n", p);
 
@@ -506,7 +507,11 @@ static void trustdom_list_done(struct tevent_req *req)
 			break;
 		}
 
-		trust_flags = (uint32_t)strtoul(q, NULL, 10);
+		trust_flags = (uint32_t)strtoul_err(q, NULL, 10, &error);
+		if (error != 0) {
+			DBG_ERR("Failed to convert trust_flags\n");
+			break;
+		}
 
 		q = strtok(NULL, "\\");
 		if (q == NULL) {
@@ -514,7 +519,11 @@ static void trustdom_list_done(struct tevent_req *req)
 			break;
 		}
 
-		trust_type = (uint32_t)strtoul(q, NULL, 10);
+		trust_type = (uint32_t)strtoul_err(q, NULL, 10, &error);
+		if (error != 0) {
+			DBG_ERR("Failed to convert trust_type\n");
+			break;
+		}
 
 		q = strtok(NULL, "\n");
 		if (q == NULL) {
@@ -522,7 +531,11 @@ static void trustdom_list_done(struct tevent_req *req)
 			break;
 		}
 
-		trust_attribs = (uint32_t)strtoul(q, NULL, 10);
+		trust_attribs = (uint32_t)strtoul_err(q, NULL, 10, &error);
+		if (error != 0) {
+			DBG_ERR("Failed to convert trust_attribs\n");
+			break;
+		}
 
 		if (!within_forest) {
 			trust_flags &= ~NETR_TRUST_FLAG_IN_FOREST;
@@ -2142,6 +2155,7 @@ bool parse_xidlist(TALLOC_CTX *mem_ctx, const char *xidstr,
 		struct unixid xid;
 		unsigned long long id;
 		char *endp;
+		int error = 0;
 
 		switch (p[0]) {
 		case 'U':
@@ -2156,8 +2170,8 @@ bool parse_xidlist(TALLOC_CTX *mem_ctx, const char *xidstr,
 
 		p += 1;
 
-		id = strtoull(p, &endp, 10);
-		if ((id == ULLONG_MAX) && (errno == ERANGE)) {
+		id = strtoull_err(p, &endp, 10, &error);
+		if (error != 0) {
 			goto fail;
 		}
 		if (*endp != '\n') {