From: Swen Schillig <swen@linux.ibm.com>
Date: Wed, 30 Jan 2019 08:31:34 +0000 (+0100)
Subject: source4: Use wrapper for string to integer conversion
X-Git-Tag: talloc-2.2.0~223
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2b2ff12e706d999ff2d84affa79c64b33cc289a7;p=thirdparty%2Fsamba.git

source4: 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>
---

diff --git a/source4/dns_server/dlz_bind9.c b/source4/dns_server/dlz_bind9.c
index 82c72111a00..070990db3cf 100644
--- a/source4/dns_server/dlz_bind9.c
+++ b/source4/dns_server/dlz_bind9.c
@@ -307,8 +307,12 @@ static bool b9_dns_type(const char *type, enum dns_record_type *dtype)
 
 #define DNS_PARSE_UINT(ret, str, sep, saveptr) do {  \
 	char *istr = strtok_r(str, sep, &saveptr); \
+	int error = 0;\
 	if ((istr) == NULL) return false; \
-	(ret) = strtoul(istr, NULL, 10); \
+	(ret) = strtoul_err(istr, NULL, 10, &error); \
+	if (error != 0) {\
+		return false;\
+	}\
 	} while (0)
 
 /*
diff --git a/source4/dsdb/common/dsdb_dn.c b/source4/dsdb/common/dsdb_dn.c
index 900c6e2df63..2119a4faaf3 100644
--- a/source4/dsdb/common/dsdb_dn.c
+++ b/source4/dsdb/common/dsdb_dn.c
@@ -84,6 +84,7 @@ struct dsdb_dn *dsdb_dn_parse_trusted(TALLOC_CTX *mem_ctx, struct ldb_context *l
 	struct ldb_val bval;
 	struct ldb_val dval;
 	char *dn_str;
+	int error = 0;
 
 	enum dsdb_dn_format dn_format = dsdb_dn_oid_to_format(dn_oid);
 
@@ -134,8 +135,8 @@ struct dsdb_dn *dsdb_dn_parse_trusted(TALLOC_CTX *mem_ctx, struct ldb_context *l
 	}
 
 	errno = 0;
-	blen = strtoul(p1, &p2, 10);
-	if (errno != 0) {
+	blen = strtoul_err(p1, &p2, 10, &error);
+	if (error != 0) {
 		DEBUG(10, (__location__ ": failed\n"));
 		goto failed;
 	}
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index fc7055d800a..d466e6ff0a9 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -3873,6 +3873,7 @@ NTSTATUS dsdb_get_extended_dn_guid(struct ldb_dn *dn, struct GUID *guid, const c
 NTSTATUS dsdb_get_extended_dn_uint64(struct ldb_dn *dn, uint64_t *val, const char *component_name)
 {
 	const struct ldb_val *v;
+	int error = 0;
 
 	v = ldb_dn_get_extended_component(dn, component_name);
 	if (v == NULL) {
@@ -3887,7 +3888,10 @@ NTSTATUS dsdb_get_extended_dn_uint64(struct ldb_dn *dn, uint64_t *val, const cha
 		memcpy(s, v->data, v->length);
 		s[v->length] = 0;
 
-		*val = strtoull(s, NULL, 0);
+		*val = strtoull_err(s, NULL, 0, &error);
+		if (error != 0) {
+			return NT_STATUS_INVALID_PARAMETER;
+		}
 	}
 	return NT_STATUS_OK;
 }
@@ -3906,6 +3910,7 @@ NTSTATUS dsdb_get_extended_dn_nttime(struct ldb_dn *dn, NTTIME *nttime, const ch
 NTSTATUS dsdb_get_extended_dn_uint32(struct ldb_dn *dn, uint32_t *val, const char *component_name)
 {
 	const struct ldb_val *v;
+	int error = 0;
 
 	v = ldb_dn_get_extended_component(dn, component_name);
 	if (v == NULL) {
@@ -3919,7 +3924,10 @@ NTSTATUS dsdb_get_extended_dn_uint32(struct ldb_dn *dn, uint32_t *val, const cha
 		char s[v->length + 1];
 		memcpy(s, v->data, v->length);
 		s[v->length] = 0;
-		*val = strtoul(s, NULL, 0);
+		*val = strtoul_err(s, NULL, 0, &error);
+		if (error != 0) {
+			return NT_STATUS_INVALID_PARAMETER;
+		}
 	}
 
 	return NT_STATUS_OK;
@@ -3973,6 +3981,7 @@ uint32_t dsdb_dn_val_rmd_flags(const struct ldb_val *val)
 	const char *p;
 	uint32_t flags;
 	char *end;
+	int error = 0;
 
 	if (val->length < 13) {
 		return 0;
@@ -3981,8 +3990,8 @@ uint32_t dsdb_dn_val_rmd_flags(const struct ldb_val *val)
 	if (!p) {
 		return 0;
 	}
-	flags = strtoul(p+11, &end, 10);
-	if (!end || *end != '>') {
+	flags = strtoul_err(p+11, &end, 10, &error);
+	if (!end || *end != '>' || error != 0) {
 		/* it must end in a > */
 		return 0;
 	}
diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
index 2a9895ae641..291876e162b 100644
--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
@@ -157,11 +157,24 @@ static int dirsync_filter_entry(struct ldb_request *req,
 	for (i = msg->num_elements - 1; i >= 0; i--) {
 		attr = dsdb_attribute_by_lDAPDisplayName(dsc->schema, msg->elements[i].name);
 		if (ldb_attr_cmp(msg->elements[i].name, "uSNChanged") == 0) {
+			int error = 0;
 			/* Read the USN it will used at the end of the filtering
 			 * to update the max USN in the cookie if we
 			 * decide to keep this entry
 			 */
-			val = strtoull((const char*)msg->elements[i].values[0].data, NULL, 0);
+			val = strtoull_err(
+				(const char*)msg->elements[i].values[0].data,
+				NULL,
+				0,
+				&error);
+			if (error != 0) {
+				ldb_set_errstring(ldb,
+						  "Failed to convert USN");
+				return ldb_module_done(dsc->req,
+						       NULL,
+						       NULL,
+						       LDB_ERR_OPERATIONS_ERROR);
+			}
 			continue;
 		}
 
diff --git a/source4/dsdb/samdb/ldb_modules/partition_metadata.c b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
index 197e7b092e0..250fecbd9e6 100644
--- a/source4/dsdb/samdb/ldb_modules/partition_metadata.c
+++ b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
@@ -36,6 +36,7 @@ static int partition_metadata_get_uint64(struct ldb_module *module,
 	TDB_DATA tdb_key, tdb_data;
 	char *value_str;
 	TALLOC_CTX *tmp_ctx;
+	int error = 0;
 
 	data = talloc_get_type_abort(ldb_module_get_private(module),
 				     struct partition_private_data);
@@ -73,7 +74,11 @@ static int partition_metadata_get_uint64(struct ldb_module *module,
 		return ldb_module_oom(module);
 	}
 
-	*value = strtoull(value_str, NULL, 10);
+	*value = strtoull_err(value_str, NULL, 10, &error);
+	if (error != 0) {
+		return ldb_module_error(module, LDB_ERR_OPERATIONS_ERROR,
+					"partition_metadata: converision failed");
+	}
 
 	SAFE_FREE(tdb_data.dptr);
 	talloc_free(tmp_ctx);
diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index e69228c32c7..ae99ebbe9ed 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -3313,6 +3313,7 @@ static int verify_cidr(const char *cidr)
 	char *address_redux = NULL;
 	unsigned int address_len;
 	TALLOC_CTX *frame = NULL;
+	int error = 0;
 
 	DBG_DEBUG("CIDR is %s\n", cidr);
 	frame = talloc_stackframe();
@@ -3329,14 +3330,14 @@ static int verify_cidr(const char *cidr)
 	/* terminate the address for strchr, inet_pton */
 	*slash = '\0';
 
-	mask = strtoul(slash + 1, &endptr, 10);
+	mask = strtoul_err(slash + 1, &endptr, 10, &error);
 	if (mask == 0){
 		DBG_INFO("Windows does not like the zero mask, "
 			 "so nor do we: %s\n", cidr);
 		goto error;
 	}
 
-	if (*endptr != '\0' || endptr == slash + 1){
+	if (*endptr != '\0' || endptr == slash + 1 || error != 0){
 		DBG_INFO("CIDR mask is not a proper integer: %s\n", cidr);
 		goto error;
 	}
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index 473a2e0a1f7..3b0781685cf 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -117,6 +117,7 @@ static int schema_metadata_get_uint64(struct schema_load_private_data *data,
 	char *value_str;
 	TALLOC_CTX *tmp_ctx;
 	int tdb_seqnum;
+	int error = 0;
 
 	if (!data) {
 		*value = default_value;
@@ -169,7 +170,12 @@ static int schema_metadata_get_uint64(struct schema_load_private_data *data,
 	 * next time
 	 */
 	data->tdb_seqnum = tdb_seqnum;
-	data->schema_seq_num_cache = strtoull(value_str, NULL, 10);
+	data->schema_seq_num_cache = strtoull_err(value_str, NULL, 10, &error);
+	if (error != 0) {
+		talloc_free(tmp_ctx);
+		return ldb_module_error(data->module, LDB_ERR_OPERATIONS_ERROR,
+					"Failed to convert value");
+	}
 
 	*value = data->schema_seq_num_cache;
 
diff --git a/source4/dsdb/schema/schema_prefixmap.c b/source4/dsdb/schema/schema_prefixmap.c
index c30aecc0ec2..482a35f2165 100644
--- a/source4/dsdb/schema/schema_prefixmap.c
+++ b/source4/dsdb/schema/schema_prefixmap.c
@@ -216,6 +216,7 @@ static WERROR _dsdb_pfm_make_binary_oid(const char *full_oid, TALLOC_CTX *mem_ct
 {
 	uint32_t last_subid;
 	const char *oid_subid;
+	int error = 0;
 
 	/* make last sub-identifier value */
 	oid_subid = strrchr(full_oid, '.');
@@ -223,7 +224,10 @@ static WERROR _dsdb_pfm_make_binary_oid(const char *full_oid, TALLOC_CTX *mem_ct
 		return WERR_INVALID_PARAMETER;
 	}
 	oid_subid++;
-	last_subid = strtoul(oid_subid, NULL, 10);
+	last_subid = strtoul_err(oid_subid, NULL, 10, &error);
+	if (error != 0) {
+		return WERR_INVALID_PARAMETER;
+	}
 
 	/* encode oid in BER format */
 	if (!ber_write_OID_String(mem_ctx, _bin_oid, full_oid)) {
diff --git a/source4/lib/registry/ldb.c b/source4/lib/registry/ldb.c
index 8bb6fd5c10f..ec293c46149 100644
--- a/source4/lib/registry/ldb.c
+++ b/source4/lib/registry/ldb.c
@@ -75,8 +75,16 @@ static void reg_ldb_unpack_value(TALLOC_CTX *mem_ctx,
 	case REG_DWORD:
 	case REG_DWORD_BIG_ENDIAN:
 		if (val != NULL) {
+			int error = 0;
 			/* The data is a plain DWORD */
-			uint32_t tmp = strtoul((char *)val->data, NULL, 0);
+			uint32_t tmp;
+
+			tmp = strtoul_err((char *)val->data, NULL, 0, &error);
+			if (error != 0) {
+				data->data = NULL;
+				data->length = 0;
+				break;
+			}
 			data->data = talloc_size(mem_ctx, sizeof(uint32_t));
 			if (data->data != NULL) {
 				SIVAL(data->data, 0, tmp);
@@ -90,8 +98,16 @@ static void reg_ldb_unpack_value(TALLOC_CTX *mem_ctx,
 
 	case REG_QWORD:
 		if (val != NULL) {
+			int error = 0;
 			/* The data is a plain QWORD */
-			uint64_t tmp = strtoull((char *)val->data, NULL, 0);
+			uint64_t tmp;
+
+			tmp = strtoull_err((char *)val->data, NULL, 0, &error);
+			if (error != 0) {
+				data->data = NULL;
+				data->length = 0;
+				break;
+			}
 			data->data = talloc_size(mem_ctx, sizeof(uint64_t));
 			if (data->data != NULL) {
 				SBVAL(data->data, 0, tmp);
diff --git a/source4/lib/socket/interface.c b/source4/lib/socket/interface.c
index 594df1a0526..494cbd4fdd2 100644
--- a/source4/lib/socket/interface.c
+++ b/source4/lib/socket/interface.c
@@ -225,8 +225,10 @@ static void interpret_interface(TALLOC_CTX *mem_ctx,
 		}
 	} else {
 		char *endp = NULL;
-		unsigned long val = strtoul(p, &endp, 0);
-		if (p == endp || (endp && *endp != '\0')) {
+		int error = 0;
+
+		unsigned long val = strtoul_err(p, &endp, 0, &error);
+		if (p == endp || (endp && *endp != '\0') || error != 0) {
 			DEBUG(2,("interpret_interface: "
 				"can't determine netmask value from %s\n",
 				p));
diff --git a/source4/libcli/resolve/dns_ex.c b/source4/libcli/resolve/dns_ex.c
index 6174b617cb3..a6863aed59e 100644
--- a/source4/libcli/resolve/dns_ex.c
+++ b/source4/libcli/resolve/dns_ex.c
@@ -514,6 +514,7 @@ static void pipe_handler(struct tevent_context *ev, struct tevent_fd *fde,
 		uint32_t port = 0;
 		char *p = strrchr(addrs[i], '@');
 		char *n;
+		int error = 0;
 
 		if (!p) {
 			composite_error(c, NT_STATUS_OBJECT_NAME_NOT_FOUND);
@@ -536,8 +537,8 @@ static void pipe_handler(struct tevent_context *ev, struct tevent_fd *fde,
 			composite_error(c, NT_STATUS_OBJECT_NAME_NOT_FOUND);
 			return;
 		}
-		port = strtoul(p, NULL, 10);
-		if (port > UINT16_MAX) {
+		port = strtoul_err(p, NULL, 10, &error);
+		if (port > UINT16_MAX || error != 0) {
 			composite_error(c, NT_STATUS_OBJECT_NAME_NOT_FOUND);
 			return;
 		}
diff --git a/source4/nbt_server/wins/winsdb.c b/source4/nbt_server/wins/winsdb.c
index fa9a6154b9a..5184d7de7b0 100644
--- a/source4/nbt_server/wins/winsdb.c
+++ b/source4/nbt_server/wins/winsdb.c
@@ -148,6 +148,7 @@ static NTSTATUS winsdb_nbt_name(TALLOC_CTX *mem_ctx, struct ldb_dn *dn, struct n
 	struct nbt_name *name;
 	unsigned int comp_num;
 	uint32_t cur = 0;
+	int error = 0;
 
 	name = talloc(mem_ctx, struct nbt_name);
 	if (!name) {
@@ -181,7 +182,16 @@ static NTSTATUS winsdb_nbt_name(TALLOC_CTX *mem_ctx, struct ldb_dn *dn, struct n
 	}
 
 	if (comp_num > cur && strcasecmp("type", ldb_dn_get_component_name(dn, cur)) == 0) {
-		name->type	= strtoul((char *)ldb_dn_get_component_val(dn, cur)->data, NULL, 0);
+		name->type =
+			strtoul_err(
+				(char *)ldb_dn_get_component_val(dn, cur)->data,
+				NULL,
+				0,
+				&error);
+		if (error != 0) {
+			status = NT_STATUS_INTERNAL_DB_CORRUPTION;
+			goto failed;
+		}
 		cur++;
 	} else {
 		status = NT_STATUS_INTERNAL_DB_CORRUPTION;
diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index cbbd9f482f2..64d2ec9991a 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -1698,6 +1698,7 @@ static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx,
 	uint32_t orig_uint = 0;
 	unsigned int flags = 0;
 	int ret;
+	int error = 0;
 
 	orig_val = ldb_msg_find_ldb_val(orig, attribute);
 	if (!orig_val || !orig_val->data) {
@@ -1705,9 +1706,11 @@ static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx,
 		flags = LDB_FLAG_MOD_ADD;
 
 	} else {
-		errno = 0;
-		orig_uint = strtoul((const char *)orig_val->data, NULL, 0);
-		if (errno != 0 || orig_uint != value) {
+		orig_uint = strtoul_err((const char *)orig_val->data,
+					NULL,
+					0,
+					&error);
+		if (error != 0 || orig_uint != value) {
 			/* replace also if can't get value */
 			flags = LDB_FLAG_MOD_REPLACE;
 		}
diff --git a/source4/torture/nbench/nbench.c b/source4/torture/nbench/nbench.c
index 8e4a984c9b5..83395123c33 100644
--- a/source4/torture/nbench/nbench.c
+++ b/source4/torture/nbench/nbench.c
@@ -96,6 +96,8 @@ again:
 	while (fgets(line, sizeof(line)-1, f)) {
 		NTSTATUS status;
 		const char **params0, **params;
+		unsigned long int tmp;
+		int error = 0;
 
 		nbench_line_count++;
 
@@ -138,7 +140,11 @@ again:
 
 		/* accept numeric or string status codes */
 		if (strncmp(params[i-1], "0x", 2) == 0) {
-			status = NT_STATUS(strtoul(params[i-1], NULL, 16));
+			tmp = strtoul_err(params[i-1], NULL, 16, &error);
+			if (error != 0) {
+				tmp = error;
+			}
+			status = NT_STATUS(tmp);
 		} else {
 			status = nt_status_string_to_code(params[i-1]);
 		}
diff --git a/source4/torture/smb2/sharemode.c b/source4/torture/smb2/sharemode.c
index 8c7403c8302..b76aa4ccd7d 100644
--- a/source4/torture/smb2/sharemode.c
+++ b/source4/torture/smb2/sharemode.c
@@ -186,12 +186,17 @@ bool torture_smb2_check_sharemode(struct torture_context *tctx)
 	struct smb2_create create = { };
 	NTSTATUS status;
 	bool ret = true;
+	int error = 0;
 
 	sharemode_string = torture_setting_string(tctx, "sharemode", "RWD");
 	sharemode = smb2_util_share_access(sharemode_string);
 
 	access_string = torture_setting_string(tctx, "access", "0xf01ff");
-	access = strtoul(access_string, NULL, 0);
+	access = strtoul_err(access_string, NULL, 0, &error);
+	if (error != 0) {
+		torture_comment(tctx, "Initializing access failed.\n");
+		return false;
+	}
 
 	filename = torture_setting_string(tctx, "filename", "testfile");
 	operation = torture_setting_string(tctx, "operation", "WD");
diff --git a/source4/web_server/web_server.c b/source4/web_server/web_server.c
index 7c2717368e3..a1db34be21a 100644
--- a/source4/web_server/web_server.c
+++ b/source4/web_server/web_server.c
@@ -107,6 +107,8 @@ void websrv_output(struct websrv_context *web, const void *data, size_t length)
 */
 NTSTATUS http_parse_header(struct websrv_context *web, const char *line)
 {
+	int error = 0;
+
 	if (line[0] == 0) {
 		web->input.end_of_headers = true;
 	} else if (strncasecmp(line,"GET ", 4)==0) {
@@ -118,7 +120,10 @@ NTSTATUS http_parse_header(struct websrv_context *web, const char *line)
 		http_error(web, "400 Bad request", "This server only accepts GET and POST requests");
 		return NT_STATUS_INVALID_PARAMETER;
 	} else if (strncasecmp(line, "Content-Length: ", 16)==0) {
-		web->input.content_length = strtoul(&line[16], NULL, 10);
+		web->input.content_length = strtoul_err(&line[16], NULL, 10, &error);
+		if (error != 0) {
+			return NT_STATUS_INVALID_PARAMETER;
+		}
 	} else {
 		struct http_header *hdr = talloc_zero(web, struct http_header);
 		char *colon = strchr(line, ':');