[PATCH] Fix string to integer conversion for occurences in source3

swen swen at linux.ibm.com
Thu Jan 10 17:54:51 UTC 2019


This is patchset is the successor of the mail thread
	[PATCH] [tldap] check for successful string conversion
which I had with Volker today.

As stated in the subject this patch-set updates all occurences of 
the usage strtoul(l) to verify the result is valid.

Please review and push if happy.

Thanks for your support in advance.

Cheers Swen

P.S.: My comment/question about strings representing negative values is
still vital...so please comment !
-------------- next part --------------
From 902b64aac9a7cbbf58982ac2cf10a9484190abe3 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Thu, 10 Jan 2019 09:44:00 +0100
Subject: [PATCH 1/7] lib: Fix string to integer conversion

In order to detect an 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.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/lib/interface.c     | 7 +++++--
 source3/lib/messages_dgm.c  | 9 ++++-----
 source3/lib/namemap_cache.c | 6 ++++--
 source3/lib/sysquotas.c     | 5 +++++
 source3/lib/time.c          | 1 +
 source3/lib/tldap_util.c    | 9 +++++++++
 source3/lib/util_str.c      | 3 ++-
 7 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/source3/lib/interface.c b/source3/lib/interface.c
index a3bc5d24e91..3c7cdc4caa6 100644
--- a/source3/lib/interface.c
+++ b/source3/lib/interface.c
@@ -516,8 +516,11 @@ static void interpret_interface(char *token)
 		}
 	} else {
 		char *endp = NULL;
-		unsigned long val = strtoul(p, &endp, 0);
-		if (p == endp || (endp && *endp != '\0')) {
+		unsigned long val;
+
+		errno = 0;
+		val = strtoul(p, &endp, 0);
+		if (p == endp || (endp && *endp != '\0') || (errno != 0)) {
 			DEBUG(2,("interpret_interface: "
 				"can't determine netmask value from %s\n",
 				p));
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index af12be8d82e..f1b81a53c5b 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -1457,13 +1457,12 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
 	}
 	buf[rw_ret] = '\0';
 
+	errno = 0;
 	unique = strtoull(buf, &endptr, 10);
-	if ((unique == 0) && (errno == EINVAL)) {
-		return EINVAL;
-	}
-	if ((unique == ULLONG_MAX) && (errno == ERANGE)) {
-		return ERANGE;
+	if (errno != 0) {
+		return errno;
 	}
+
 	if (endptr[0] != '\n') {
 		return EINVAL;
 	}
diff --git a/source3/lib/namemap_cache.c b/source3/lib/namemap_cache.c
index fa179517f9f..677eaf2c509 100644
--- a/source3/lib/namemap_cache.c
+++ b/source3/lib/namemap_cache.c
@@ -123,8 +123,9 @@ static void namemap_cache_find_sid_parser(
 		return;
 	}
 
+	errno = 0;
 	type = strtoul(typebuf, &endptr, 10);
-	if (*endptr != '\0') {
+	if ((*endptr != '\0') || (errno != 0)) {
 		return;
 	}
 	if ((type == ULONG_MAX) && (errno == ERANGE)) {
@@ -276,8 +277,9 @@ static void namemap_cache_find_name_parser(
 		return;
 	}
 
+	errno = 0;
 	type = strtoul(typebuf, &endptr, 10);
-	if (*endptr != '\0') {
+	if ((*endptr != '\0') || (errno != 0)) {
 		return;
 	}
 	if ((type == ULONG_MAX) && (errno == ERANGE)) {
diff --git a/source3/lib/sysquotas.c b/source3/lib/sysquotas.c
index 9b2d37b8375..6a7547d450f 100644
--- a/source3/lib/sysquotas.c
+++ b/source3/lib/sysquotas.c
@@ -282,7 +282,12 @@ static int command_get_quota(const char *path, enum SMB_QUOTA_TYPE qtype, unid_t
 
 			/* we need to deal with long long unsigned here, if supported */
 
+			errno = 0;
 			dp->qflags = strtoul(line, &p2, 10);
+			if (errno != 0) {
+				goto invalid_param;
+			}
+
 			p = p2;
 			while (p && *p && isspace(*p)) {
 				p++;
diff --git a/source3/lib/time.c b/source3/lib/time.c
index 30ad1ec9a01..09c5b0656f6 100644
--- a/source3/lib/time.c
+++ b/source3/lib/time.c
@@ -42,6 +42,7 @@
 */
 NTTIME nttime_from_string(const char *s)
 {
+	errno = 0;
 	return strtoull(s, NULL, 0);
 }
 
diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c
index 508c6c02f80..d8a65c4e2d8 100644
--- a/source3/lib/tldap_util.c
+++ b/source3/lib/tldap_util.c
@@ -395,7 +395,16 @@ bool tldap_pull_uint64(struct tldap_message *msg, const char *attr,
 		DEBUG(10, ("Could not find attribute %s\n", attr));
 		return false;
 	}
+
+	errno = 0;
 	result = strtoull(str, NULL, 10);
+	if (errno != 0) {
+		DBG_DEBUG("Attribute conversion failed (%s)\n",
+			  strerror(errno));
+		TALLOC_FREE(str);
+		return false;
+	}
+
 	TALLOC_FREE(str);
 	*presult = result;
 	return true;
diff --git a/source3/lib/util_str.c b/source3/lib/util_str.c
index 8568af46c17..920c1732782 100644
--- a/source3/lib/util_str.c
+++ b/source3/lib/util_str.c
@@ -856,9 +856,10 @@ uint64_t conv_str_size(const char * str)
                 return 0;
         }
 
+	errno = 0;
 	lval = strtoull(str, &end, 10 /* base */);
 
-        if (end == NULL || end == str) {
+        if (end == NULL || end == str || errno != 0) {
                 return 0;
         }
 
-- 
2.20.1


From 8da1825c141bf1baa3d12a67492cc95e469828f0 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Thu, 10 Jan 2019 17:44:44 +0100
Subject: [PATCH 2/7] groupdb: Fix string to integer conversion

In order to detect an 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.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/groupdb/mapping.c     | 5 +++++
 source3/groupdb/mapping_tdb.c | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/source3/groupdb/mapping.c b/source3/groupdb/mapping.c
index 43722e777d4..77ea86d5bef 100644
--- a/source3/groupdb/mapping.c
+++ b/source3/groupdb/mapping.c
@@ -243,8 +243,13 @@ int smb_create_group(const char *unix_group, gid_t *new_gid)
 
 			nread = read(fd, output, sizeof(output)-1);
 			if (nread > 0) {
+				errno = 0;
 				output[nread] = '\0';
 				*new_gid = (gid_t)strtoul(output, NULL, 10);
+				if (errno != 0) {
+					*new_gid = 0;
+					return errno;
+				}
 			}
 
 			close(fd);
diff --git a/source3/groupdb/mapping_tdb.c b/source3/groupdb/mapping_tdb.c
index d6a06ef199b..391ee9e8d1f 100644
--- a/source3/groupdb/mapping_tdb.c
+++ b/source3/groupdb/mapping_tdb.c
@@ -974,8 +974,9 @@ static int convert_ldb_record(TDB_CONTEXT *ltdb, TDB_DATA key,
 			/* we ignore unknown or uninteresting attributes
 			 * (objectclass, etc.) */
 			if (strcasecmp_m(name, "gidNumber") == 0) {
+				errno = 0;
 				map->gid = strtoul(val, &q, 10);
-				if (*q) {
+				if (*q || errno != 0) {
 					errno = EIO;
 					goto failed;
 				}
@@ -985,8 +986,9 @@ static int convert_ldb_record(TDB_CONTEXT *ltdb, TDB_DATA key,
 					goto failed;
 				}
 			} else if (strcasecmp_m(name, "sidNameUse") == 0) {
+				errno = 0;
 				map->sid_name_use = strtoul(val, &q, 10);
-				if (*q) {
+				if (*q || errno != 0) {
 					errno = EIO;
 					goto failed;
 				}
-- 
2.20.1


From 5f27a2ef011bdf8dc18a759c85c25bbf30657b5e Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Thu, 10 Jan 2019 17:57:45 +0100
Subject: [PATCH 3/7] utils: Fix string to integer conversion

In order to detect an 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.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/utils/net_idmap.c        |  3 ++-
 source3/utils/net_registry.c     | 14 +++++++++++++-
 source3/utils/net_rpc_registry.c |  9 ++++++++-
 source3/utils/net_sam.c          |  5 ++++-
 source3/utils/pdbedit.c          |  9 +++++++--
 source3/utils/regedit_dialog.c   |  8 ++++++--
 6 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/source3/utils/net_idmap.c b/source3/utils/net_idmap.c
index b49d5f43381..e4af80a9e0b 100644
--- a/source3/utils/net_idmap.c
+++ b/source3/utils/net_idmap.c
@@ -629,6 +629,7 @@ static bool parse_uint32(const char *str, uint32_t *result)
 	unsigned long val;
 	char *endptr;
 
+	errno = 0;
 	val = strtoul(str, &endptr, 10);
 
 	if (str == endptr) {
@@ -637,7 +638,7 @@ static bool parse_uint32(const char *str, uint32_t *result)
 	if (*endptr != '\0') {
 		return false;
 	}
-	if ((val == ULONG_MAX) && (errno == ERANGE)) {
+	if (errno != 0) {
 		return false;
 	}
 	if ((val & UINT32_MAX) != val) {
diff --git a/source3/utils/net_registry.c b/source3/utils/net_registry.c
index 01a36b20e7c..beb35e0d47c 100644
--- a/source3/utils/net_registry.c
+++ b/source3/utils/net_registry.c
@@ -509,7 +509,14 @@ static int net_registry_setvalue(struct net_context *c, int argc,
 	}
 
 	if (strequal(argv[2], "dword")) {
-		uint32_t v = strtoul(argv[3], NULL, 10);
+		uint32_t v;
+
+		errno = 0;
+		v = strtoul(argv[3], NULL, 10);
+		if (errno != 0) {
+			ret = errno;
+			goto done;
+		}
 		value.type = REG_DWORD;
 		value.data = data_blob_talloc(ctx, NULL, 4);
 		SIVAL(value.data.data, 0, v);
@@ -641,7 +648,12 @@ static int net_registry_increment(struct net_context *c, int argc,
 
 	state.increment = 1;
 	if (argc == 3) {
+		errno = 0;
 		state.increment = strtoul(argv[2], NULL, 10);
+		if (errno != 0) {
+			ret = errno;
+			goto done;
+		}
 	}
 
 	status = g_lock_do(string_term_tdb_data("registry_increment_lock"),
diff --git a/source3/utils/net_rpc_registry.c b/source3/utils/net_rpc_registry.c
index 19b73fd7042..29c8fcd8868 100644
--- a/source3/utils/net_rpc_registry.c
+++ b/source3/utils/net_rpc_registry.c
@@ -603,7 +603,14 @@ static NTSTATUS rpc_registry_setvalue_internal(struct net_context *c,
 	}
 
 	if (strequal(argv[2], "dword")) {
-		uint32_t v = strtoul(argv[3], NULL, 10);
+		uint32_t v;
+
+		errno = 0;
+		v = strtoul(argv[3], NULL, 10);
+		if (errno != 0) {
+			goto error;
+		}
+
 		value.type = REG_DWORD;
 		value.data = data_blob_talloc(mem_ctx, NULL, 4);
 		SIVAL(value.data.data, 0, v);
diff --git a/source3/utils/net_sam.c b/source3/utils/net_sam.c
index 5a9d1792d51..5cf9565f397 100644
--- a/source3/utils/net_sam.c
+++ b/source3/utils/net_sam.c
@@ -500,9 +500,12 @@ static int net_sam_policy_set(struct net_context *c, int argc, const char **argv
 		value = -1;
 	}
 	else {
+		errno = 0;
 		value = strtoul(argv[1], &endptr, 10);
 
-		if ((endptr == argv[1]) || (endptr[0] != '\0')) {
+		if ((endptr == argv[1]) ||
+		    (endptr[0] != '\0') ||
+		    (errno != 0)) {
 			d_printf(_("Unable to set policy \"%s\"! Invalid value "
 				 "\"%s\".\n"),
 				 account_policy, argv[1]);
diff --git a/source3/utils/pdbedit.c b/source3/utils/pdbedit.c
index 585f1bfada3..478a5892644 100644
--- a/source3/utils/pdbedit.c
+++ b/source3/utils/pdbedit.c
@@ -598,9 +598,14 @@ static int set_user_info(const char *username, const char *fullname,
 		time_t value = get_time_t_max();
 
 		if (strcmp(kickoff_time, "never") != 0) {
-			uint32_t num = strtoul(kickoff_time, &endptr, 10);
+			uint32_t num;
 
-			if ((endptr == kickoff_time) || (endptr[0] != '\0')) {
+			errno = 0;
+			num = strtoul(kickoff_time, &endptr, 10);
+
+			if ((endptr == kickoff_time) ||
+			    (endptr[0] != '\0') ||
+			    (errno != 0)) {
 				fprintf(stderr, "Failed to parse kickoff time\n");
 				return -1;
 			}
diff --git a/source3/utils/regedit_dialog.c b/source3/utils/regedit_dialog.c
index dcce66bf208..c1b3cda5068 100644
--- a/source3/utils/regedit_dialog.c
+++ b/source3/utils/regedit_dialog.c
@@ -1032,6 +1032,7 @@ bool dialog_section_text_field_get_uint(struct dialog_section *section,
 	bool rv;
 	const char *buf;
 	char *endp;
+	unsigned long long tmp_out;
 	struct dialog_section_text_field *text_field =
 		talloc_get_type_abort(section, struct dialog_section_text_field);
 
@@ -1041,12 +1042,15 @@ bool dialog_section_text_field_get_uint(struct dialog_section *section,
 	if (buf == NULL) {
 		return false;
 	}
-	*out = strtoull(buf, &endp, 0);
+
+	errno = 0;
+	tmp_out = strtoull(buf, &endp, 0);
 	rv = true;
-	if (endp == buf || endp == NULL || endp[0] != '\0') {
+	if (endp == buf || endp == NULL || endp[0] != '\0' || errno != 0) {
 		rv = false;
 	}
 
+	*out = tmp_out;
 	return rv;
 }
 
-- 
2.20.1


From 1db9fe6391049e555c3b7e12ae402ae6ddce2a1e Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Thu, 10 Jan 2019 18:24:16 +0100
Subject: [PATCH 4/7] passdb: Fix string to integer conversion

In order to detect an 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.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/passdb/account_pol.c |  8 ++++-
 source3/passdb/pdb_ldap.c    | 61 +++++++++++++++++++++++++++++++-----
 source3/passdb/pdb_tdb.c     |  4 +++
 3 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/source3/passdb/account_pol.c b/source3/passdb/account_pol.c
index e566eca78eb..0d748a12dfd 100644
--- a/source3/passdb/account_pol.c
+++ b/source3/passdb/account_pol.c
@@ -456,7 +456,13 @@ bool cache_account_policy_get(enum pdb_policy_type type, uint32_t *value)
 	}
 
 	if (gencache_get(cache_key, talloc_tos(), &cache_value, NULL)) {
-		uint32_t tmp = strtoul(cache_value, NULL, 10);
+		uint32_t tmp;
+
+		errno = 0;
+		tmp = strtoul(cache_value, NULL, 10);
+		if (errno != 0) {
+			goto done;
+		}
 		*value = tmp;
 		ret = True;
 	}
diff --git a/source3/passdb/pdb_ldap.c b/source3/passdb/pdb_ldap.c
index 7f8903ba96d..238a18b13e3 100644
--- a/source3/passdb/pdb_ldap.c
+++ b/source3/passdb/pdb_ldap.c
@@ -995,7 +995,12 @@ static bool init_sam_from_ldap(struct ldapsam_privates *ldap_state,
 				ctx);
 		if (temp) {
 			/* We've got a uid, feed the cache */
+			errno = 0;
 			unix_pw.pw_uid = strtoul(temp, NULL, 10);
+			if (errno != 0) {
+				DBG_ERR("Failed to convert UID\n");
+				goto fn_exit;
+			}
 			have_uid = true;
 		}
 		temp = smbldap_talloc_single_attribute(
@@ -1005,7 +1010,12 @@ static bool init_sam_from_ldap(struct ldapsam_privates *ldap_state,
 				ctx);
 		if (temp) {
 			/* We've got a uid, feed the cache */
+			errno = 0;
 			unix_pw.pw_gid = strtoul(temp, NULL, 10);
+			if (errno != 0) {
+				DBG_ERR("Failed to convert GID\n");
+				goto fn_exit;
+			}
 			have_gid = true;
 		}
 		unix_pw.pw_gecos = smbldap_talloc_single_attribute(
@@ -2928,7 +2938,12 @@ static NTSTATUS ldapsam_enum_group_memberships(struct pdb_methods *methods,
 				ret = NT_STATUS_INTERNAL_DB_CORRUPTION;
 				goto done;
 			}
+			errno = 0;
 			primary_gid = strtoul(gidstr, NULL, 10);
+			if (errno != 0) {
+				DBG_ERR("Failed to convert GID\n");
+				goto done;
+			}
 			break;
 		default:
 			DEBUG(1, ("found more than one account with the same user name ?!\n"));
@@ -2996,9 +3011,10 @@ static NTSTATUS ldapsam_enum_group_memberships(struct pdb_methods *methods,
 						  str, sizeof(str)-1))
 			continue;
 
+		errno = 0;
 		gid = strtoul(str, &end, 10);
 
-		if (PTR_DIFF(end, str) != strlen(str))
+		if (PTR_DIFF(end, str) != strlen(str) || errno != 0)
 			goto done;
 
 		if (gid == primary_gid) {
@@ -4923,6 +4939,7 @@ static NTSTATUS ldapsam_get_new_rid(struct ldapsam_privates *priv,
 	char *value;
 	int rc;
 	uint32_t nextRid = 0;
+	uint32_t tmp;
 	const char *dn;
 
 	TALLOC_CTX *mem_ctx;
@@ -4959,21 +4976,33 @@ static NTSTATUS ldapsam_get_new_rid(struct ldapsam_privates *priv,
 	value = smbldap_talloc_single_attribute(priv2ld(priv), entry,
 						"sambaNextRid",	mem_ctx);
 	if (value != NULL) {
-		uint32_t tmp = (uint32_t)strtoul(value, NULL, 10);
+		errno = 0;
+		tmp = (uint32_t)strtoul(value, NULL, 10);
+		if (errno != 0) {
+			goto done;
+		}
 		nextRid = MAX(nextRid, tmp);
 	}
 
 	value = smbldap_talloc_single_attribute(priv2ld(priv), entry,
 						"sambaNextUserRid", mem_ctx);
 	if (value != NULL) {
-		uint32_t tmp = (uint32_t)strtoul(value, NULL, 10);
+		errno = 0;
+		tmp = (uint32_t)strtoul(value, NULL, 10);
+		if (errno != 0) {
+			goto done;
+		}
 		nextRid = MAX(nextRid, tmp);
 	}
 
 	value = smbldap_talloc_single_attribute(priv2ld(priv), entry,
 						"sambaNextGroupRid", mem_ctx);
 	if (value != NULL) {
-		uint32_t tmp = (uint32_t)strtoul(value, NULL, 10);
+		errno = 0;
+		tmp = (uint32_t)strtoul(value, NULL, 10);
+		if (errno != 0) {
+			goto done;
+		}
 		nextRid = MAX(nextRid, tmp);
 	}
 
@@ -5048,6 +5077,7 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
 				NULL };
 	LDAPMessage *result = NULL;
 	LDAPMessage *entry = NULL;
+	unsigned long tmp_id;
 	bool ret = False;
 	char *value;
 	int rc;
@@ -5106,7 +5136,13 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
 			goto done;
 		}
 
-		id->id = strtoul(gid_str, NULL, 10);
+		errno = 0;
+		tmp_id = strtoul(gid_str, NULL, 10);
+		if (errno != 0) {
+			goto done;
+		}
+
+		id->id = tmp_id;
 		id->type = ID_TYPE_GID;
 		ret = True;
 		goto done;
@@ -5122,7 +5158,12 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
 		goto done;
 	}
 
-	id->id = strtoul(value, NULL, 10);
+	errno = 0;
+	tmp_id = strtoul(value, NULL, 10);
+	if (errno != 0) {
+		goto done;
+	}
+	id->id = tmp_id;
 	id->type = ID_TYPE_UID;
 
 	ret = True;
@@ -5709,7 +5750,12 @@ static NTSTATUS ldapsam_create_dom_group(struct pdb_methods *my_methods,
 			return NT_STATUS_INTERNAL_DB_CORRUPTION;
 		}
 
+		errno = 0;
 		gid = strtoul(tmp, NULL, 10);
+		if (errno != 0) {
+			DBG_ERR("Failed to convert gidNumber\n");
+			return NT_STATUS_UNSUCCESSFUL;
+		}
 
 		dn = smbldap_talloc_dn(tmp_ctx, priv2ld(ldap_state), entry);
 		if (!dn) {
@@ -5981,9 +6027,10 @@ static NTSTATUS ldapsam_change_groupmem(struct pdb_methods *my_methods,
 			return NT_STATUS_INTERNAL_DB_CORRUPTION;
 		}
 
+		errno = 0;
 		user_gid = strtoul(gidstr, NULL, 10);
 
-		if (!sid_to_gid(&group_sid, &group_gid)) {
+		if (!sid_to_gid(&group_sid, &group_gid) || errno != 0) {
 			DEBUG (0, ("ldapsam_change_groupmem: Unable to get group gid from SID!\n"));
 			return NT_STATUS_UNSUCCESSFUL;
 		}
diff --git a/source3/passdb/pdb_tdb.c b/source3/passdb/pdb_tdb.c
index 91735ff7084..963732bfaa6 100644
--- a/source3/passdb/pdb_tdb.c
+++ b/source3/passdb/pdb_tdb.c
@@ -1182,7 +1182,11 @@ static int tdbsam_collect_rids(struct db_record *rec, void *private_data)
 		return 0;
 	}
 
+	errno = 0;
 	rid = strtoul((char *)key.dptr+prefixlen, NULL, 16);
+	if (errno != 0) {
+		return 0;
+	}
 
 	ADD_TO_LARGE_ARRAY(state, uint32_t, rid, &state->rids, &state->num_rids,
 			   &state->array_size);
-- 
2.20.1


From 3dcd271aabd180e78b649a6c5cb264c25eee5590 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Thu, 10 Jan 2019 18:37:35 +0100
Subject: [PATCH 5/7] winbindd: Fix string to integer conversion

In order to detect an 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.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/winbindd/idmap_ldap.c          | 11 +++++++++--
 source3/winbindd/winbindd_lookuprids.c |  4 +++-
 source3/winbindd/winbindd_util.c       | 18 +++++++++++++++++-
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/source3/winbindd/idmap_ldap.c b/source3/winbindd/idmap_ldap.c
index 17cc7404f12..c73aac40bd7 100644
--- a/source3/winbindd/idmap_ldap.c
+++ b/source3/winbindd/idmap_ldap.c
@@ -299,7 +299,12 @@ static NTSTATUS idmap_ldap_allocate_id_internal(struct idmap_domain *dom,
 		goto done;
 	}
 
+	errno = 0;
 	xid->id = strtoul(id_str, NULL, 10);
+	if (errno != 0) {
+		ret = NT_STATUS_UNSUCCESSFUL;
+		goto done;
+	}
 
 	/* make sure we still have room to grow */
 
@@ -769,8 +774,9 @@ again:
 			continue;
 		}
 
+		errno = 0;
 		id = strtoul(tmp, NULL, 10);
-		if (!idmap_unix_id_is_in_range(id, dom)) {
+		if (errno != 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));
@@ -1005,8 +1011,9 @@ again:
 			continue;
 		}
 
+		errno = 0;
 		id = strtoul(tmp, NULL, 10);
-		if (!idmap_unix_id_is_in_range(id, dom)) {
+		if (errno != 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));
diff --git a/source3/winbindd/winbindd_lookuprids.c b/source3/winbindd/winbindd_lookuprids.c
index 1e80b78a92e..b2982bf57b6 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;
+
+		errno = 0;
 		rids[i] = strtoul(p, &q, 10);
-		if (*q != '\n') {
+		if (errno != 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..4508d96dba9 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -506,7 +506,12 @@ static void trustdom_list_done(struct tevent_req *req)
 			break;
 		}
 
+		errno = 0;
 		trust_flags = (uint32_t)strtoul(q, NULL, 10);
+		if (errno != 0) {
+			DBG_ERR("Failed to convert trust_flags\n");
+			break;
+		}
 
 		q = strtok(NULL, "\\");
 		if (q == NULL) {
@@ -514,7 +519,12 @@ static void trustdom_list_done(struct tevent_req *req)
 			break;
 		}
 
+		errno = 0;
 		trust_type = (uint32_t)strtoul(q, NULL, 10);
+		if (errno != 0) {
+			DBG_ERR("Failed to convert trust_type\n");
+			break;
+		}
 
 		q = strtok(NULL, "\n");
 		if (q == NULL) {
@@ -522,7 +532,12 @@ static void trustdom_list_done(struct tevent_req *req)
 			break;
 		}
 
+		errno = 0;
 		trust_attribs = (uint32_t)strtoul(q, NULL, 10);
+		if (errno != 0) {
+			DBG_ERR("Failed to convert trust_attribs\n");
+			break;
+		}
 
 		if (!within_forest) {
 			trust_flags &= ~NETR_TRUST_FLAG_IN_FOREST;
@@ -2156,8 +2171,9 @@ bool parse_xidlist(TALLOC_CTX *mem_ctx, const char *xidstr,
 
 		p += 1;
 
+		errno = 0;
 		id = strtoull(p, &endp, 10);
-		if ((id == ULLONG_MAX) && (errno == ERANGE)) {
+		if (errno != 0) {
 			goto fail;
 		}
 		if (*endp != '\n') {
-- 
2.20.1


From 6caa31a6dccb2c007ccaca472140716db338546b Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Thu, 10 Jan 2019 18:41:23 +0100
Subject: [PATCH 6/7] modules: Fix string to integer conversion

In order to detect an 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.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/modules/vfs_preopen.c | 4 ++++
 source3/modules/vfs_snapper.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/source3/modules/vfs_preopen.c b/source3/modules/vfs_preopen.c
index 84b13556195..a87a7075ff3 100644
--- a/source3/modules/vfs_preopen.c
+++ b/source3/modules/vfs_preopen.c
@@ -363,7 +363,11 @@ static bool preopen_parse_fname(const char *fname, unsigned long *pnum,
 		return false;
 	}
 
+	errno = 0;
 	num = strtoul(p, (char **)&q, 10);
+	if (errno != 0) {
+		return false;
+	}
 
 	if (num+1 < num) {
 		/* overflow */
diff --git a/source3/modules/vfs_snapper.c b/source3/modules/vfs_snapper.c
index 443a940b17a..c641f0d0618 100644
--- a/source3/modules/vfs_snapper.c
+++ b/source3/modules/vfs_snapper.c
@@ -1249,9 +1249,10 @@ static NTSTATUS snapper_snap_path_to_id(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
+	errno = 0;
 	str_idx++;
 	snap_id = strtoul(str_idx, &str_end, 10);
-	if (str_idx == str_end) {
+	if (str_idx == str_end || errno != 0) {
 		talloc_free(path_dup);
 		return NT_STATUS_INVALID_PARAMETER;
 	}
-- 
2.20.1


From 565e7f9afc06da335bcb046b0836f25b1720d947 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Thu, 10 Jan 2019 18:48:08 +0100
Subject: [PATCH 7/7] rpcclient: Fix string to integer conversion

In order to detect an 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.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/rpcclient/cmd_samr.c    | 4 ++++
 source3/rpcclient/cmd_spoolss.c | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/source3/rpcclient/cmd_samr.c b/source3/rpcclient/cmd_samr.c
index d55d1648c91..7f7a6604682 100644
--- a/source3/rpcclient/cmd_samr.c
+++ b/source3/rpcclient/cmd_samr.c
@@ -1412,7 +1412,11 @@ static NTSTATUS cmd_samr_delete_alias(struct rpc_pipe_client *cli,
 		return NT_STATUS_OK;
 	}
 
+	errno = 0;
 	alias_rid = strtoul(argv[2], NULL, 10);
+	if (errno != 0) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
 
 	/* Open SAMR handle */
 
diff --git a/source3/rpcclient/cmd_spoolss.c b/source3/rpcclient/cmd_spoolss.c
index 8d330afdeb0..3475c391543 100644
--- a/source3/rpcclient/cmd_spoolss.c
+++ b/source3/rpcclient/cmd_spoolss.c
@@ -2707,7 +2707,12 @@ static WERROR cmd_spoolss_setprinterdata(struct rpc_pipe_client *cli,
 		W_ERROR_HAVE_NO_MEMORY(data.string);
 		break;
 	case REG_DWORD:
+		errno = 0;
 		data.value = strtoul(argv[4], NULL, 10);
+		if (errno != 0) {
+			result = WERR_INVALID_PARAMETER;
+			goto done;
+		}
 		break;
 	case REG_BINARY:
 		data.binary = strhex_to_data_blob(mem_ctx, argv[4]);
-- 
2.20.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190110/fc17845d/signature.sig>


More information about the samba-technical mailing list