Empy or false string conversion (was :fix string to integer conversion)

swen swen at linux.ibm.com
Tue Mar 19 09:15:22 UTC 2019


Updated patch-set as discussed.
Started pipeline run at
https://gitlab.com/samba-team/devel/samba/pipelines/52515071

Please review and push if happy.

Thanks for your support

Cheers Swen
On Tue, 2019-03-19 at 09:47 +0100, Ralph Böhme wrote:
> On Tue, Mar 19, 2019 at 09:32:41AM +0100, swen wrote:
> > Misread your update regarding the re-order.
> > Agree to your change.
> 
> ok.


-------------- next part --------------
From 6dc35a8155284c3c69e7089193fd417422ea9436 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 09:03:27 +0100
Subject: [PATCH 1/9] lib: modify string conversion wrapper to handle invalid
 strings

The standard string conversion routines convert a "non-number string"
to zero and do not flag an error.
This is changed now by returning ENODATA if no conversion occured.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 lib/util/util.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lib/util/util.c b/lib/util/util.c
index 19824b550c0..4044dd3a473 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -60,6 +60,7 @@
  * - wrong base
  * - value overflow
  * - string with a leading "-" indicating a negative number
+ * - no conversion due to empty string or not representing a number
  */
 unsigned long int
 strtoul_err(const char *nptr, char **endptr, int base, int *err)
@@ -84,6 +85,13 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
 		return val;
 	}
 
+	/* got an invalid number-string resulting in no conversion */
+	if (nptr == tmp_endptr) {
+		*err = EINVAL;
+		errno = saved_errno;
+		return val;
+	}
+
 	/* did we convert a negative "number" ? */
 	needle = strchr(nptr, '-');
 	if (needle != NULL && needle < tmp_endptr) {
@@ -107,6 +115,7 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
  * - wrong base
  * - value overflow
  * - string with a leading "-" indicating a negative number
+ * - no conversion due to empty string or not representing a number
  */
 unsigned long long int
 strtoull_err(const char *nptr, char **endptr, int base, int *err)
@@ -131,6 +140,13 @@ strtoull_err(const char *nptr, char **endptr, int base, int *err)
 		return val;
 	}
 
+	/* got an invalid number-string resulting in no conversion */
+	if (nptr == tmp_endptr) {
+		*err = EINVAL;
+		errno = saved_errno;
+		return val;
+	}
+
 	/* did we convert a negative "number" ? */
 	needle = strchr(nptr, '-');
 	if (needle != NULL && needle < tmp_endptr) {
-- 
2.20.1


From 5d3f4412f7f8f9aac184b23d349580cf8091ae1a Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 09:07:13 +0100
Subject: [PATCH 2/9] lib: Update error check for new string conversion wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/lib/interface.c    | 2 +-
 source3/lib/messages_dgm.c | 5 +----
 source3/lib/util_str.c     | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/source3/lib/interface.c b/source3/lib/interface.c
index 342c92a61a2..31066b8b5cc 100644
--- a/source3/lib/interface.c
+++ b/source3/lib/interface.c
@@ -527,7 +527,7 @@ static void interpret_interface(char *token)
 		unsigned long val;
 
 		val = strtoul_err(p, &endp, 0, &error);
-		if (p == endp || (endp && *endp != '\0') || error != 0) {
+		if (error != 0 || *endp != '\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 d73a6ad6a7c..c1059652cf1 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -1470,10 +1470,6 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
 	buf[rw_ret] = '\0';
 
 	unique = strtoull_err(buf, &endptr, 10, &error);
-	if ((unique == 0) && (errno == EINVAL)) {
-		return EINVAL;
-	}
-
 	if (error != 0) {
 		return error;
 	}
@@ -1481,6 +1477,7 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
 	if (endptr[0] != '\n') {
 		return EINVAL;
 	}
+
 	*punique = unique;
 	return 0;
 }
diff --git a/source3/lib/util_str.c b/source3/lib/util_str.c
index a0095d23978..1a27227fe41 100644
--- a/source3/lib/util_str.c
+++ b/source3/lib/util_str.c
@@ -859,7 +859,7 @@ uint64_t conv_str_size(const char * str)
 
 	lval = strtoull_err(str, &end, 10, &error);
 
-        if (end == NULL || end == str || error != 0) {
+        if (error != 0) {
                 return 0;
         }
 
-- 
2.20.1


From c1285ca68064310d7a65a697cbb5738a012abec8 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 09:29:13 +0100
Subject: [PATCH 3/9] utils: Update error check for new string conversion
 wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/utils/net_idmap.c      | 10 ++--------
 source3/utils/net_sam.c        |  2 +-
 source3/utils/pdbedit.c        |  5 +----
 source3/utils/regedit_dialog.c |  8 +++-----
 4 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/source3/utils/net_idmap.c b/source3/utils/net_idmap.c
index f6c9f3a1a3d..d1a6db95921 100644
--- a/source3/utils/net_idmap.c
+++ b/source3/utils/net_idmap.c
@@ -631,16 +631,10 @@ static bool parse_uint32(const char *str, uint32_t *result)
 	int error = 0;
 
 	val = strtoul_err(str, &endptr, 10, &error);
-
-	if (str == endptr) {
-		return false;
-	}
-	if (*endptr != '\0') {
-		return false;
-	}
-	if (error != 0) {
+	if (error != 0 || *endptr != '\0') {
 		return false;
 	}
+
 	*result = val;		/* Potential crop */
 	return true;
 }
diff --git a/source3/utils/net_sam.c b/source3/utils/net_sam.c
index fc2a7baacef..164d9408c56 100644
--- a/source3/utils/net_sam.c
+++ b/source3/utils/net_sam.c
@@ -503,7 +503,7 @@ static int net_sam_policy_set(struct net_context *c, int argc, const char **argv
 	else {
 		value = strtoul_err(argv[1], &endptr, 10, &err);
 
-		if ((endptr == argv[1]) || (endptr[0] != '\0') || (err != 0)) {
+		if (err != 0 || *endptr != '\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 c80d5411b00..462c753217e 100644
--- a/source3/utils/pdbedit.c
+++ b/source3/utils/pdbedit.c
@@ -602,10 +602,7 @@ static int set_user_info(const char *username, const char *fullname,
 			uint32_t num;
 
 			num = strtoul_err(kickoff_time, &endptr, 10, &error);
-			if ((endptr == kickoff_time) ||
-			    (endptr[0] != '\0') ||
-			    (error != 0))
-			{
+			if (error != 0 || *endptr != '\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 aeea70ac22e..7dd9f0fadea 100644
--- a/source3/utils/regedit_dialog.c
+++ b/source3/utils/regedit_dialog.c
@@ -1029,7 +1029,6 @@ bool dialog_section_text_field_get_int(struct dialog_section *section,
 bool dialog_section_text_field_get_uint(struct dialog_section *section,
 				        unsigned long long *out)
 {
-	bool rv;
 	const char *buf;
 	char *endp;
 	int error = 0;
@@ -1043,12 +1042,11 @@ bool dialog_section_text_field_get_uint(struct dialog_section *section,
 		return false;
 	}
 	*out = strtoull_err(buf, &endp, 0, &error);
-	rv = true;
-	if (endp == buf || endp == NULL || endp[0] != '\0' || error != 0) {
-		rv = false;
+	if (error != 0 || *endp != '\0') {
+		return false;
 	}
 
-	return rv;
+	return true;
 }
 
 /* hex editor field */
-- 
2.20.1


From df0f9f954b43d79ada1f157f9e50a09883703769 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 09:34:10 +0100
Subject: [PATCH 4/9] modules: Update error check for new string conversion
 wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/modules/vfs_snapper.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_snapper.c b/source3/modules/vfs_snapper.c
index 0f52f9e71c5..fdd99a2cca5 100644
--- a/source3/modules/vfs_snapper.c
+++ b/source3/modules/vfs_snapper.c
@@ -1216,7 +1216,6 @@ static NTSTATUS snapper_snap_path_to_id(TALLOC_CTX *mem_ctx,
 {
 	char *path_dup;
 	char *str_idx;
-	char *str_end;
 	uint32_t snap_id;
 	int error = 0;
 
@@ -1251,8 +1250,8 @@ static NTSTATUS snapper_snap_path_to_id(TALLOC_CTX *mem_ctx,
 	}
 
 	str_idx++;
-	snap_id = strtoul_err(str_idx, &str_end, 10, &error);
-	if (error != 0 || str_idx == str_end) {
+	snap_id = strtoul_err(str_idx, NULL, 10, &error);
+	if (error != 0) {
 		talloc_free(path_dup);
 		return NT_STATUS_INVALID_PARAMETER;
 	}
-- 
2.20.1


From d81b4c2409cc9d520f51b04b568890b299fbf4db Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 09:43:53 +0100
Subject: [PATCH 5/9] ctdb-protocol: Update error check for new string
 conversion wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 ctdb/protocol/protocol_util.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ctdb/protocol/protocol_util.c b/ctdb/protocol/protocol_util.c
index 99dbe82404d..1dfa49b9dc2 100644
--- a/ctdb/protocol/protocol_util.c
+++ b/ctdb/protocol/protocol_util.c
@@ -288,7 +288,7 @@ int ctdb_sock_addr_from_string(const char *str,
 	}
 
 	port = strtoul_err(p+1, &endp, 10, &ret);
-	if (endp == p+1 || *endp != '\0' || ret != 0) {
+	if (ret != 0 || *endp != '\0') {
 		/* Empty string or trailing garbage */
 		return EINVAL;
 	}
@@ -327,7 +327,7 @@ int ctdb_sock_addr_mask_from_string(const char *str,
 	}
 
 	m = strtoul_err(p+1, &endp, 10, &ret);
-	if (endp == p+1 || *endp != '\0' || ret != 0) {
+	if (ret != 0 || *endp != '\0') {
 		/* Empty string or trailing garbage */
 		return EINVAL;
 	}
-- 
2.20.1


From 32b4fcd44caf8e4c8b5d682d92eeb9086ddabb9a Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 09:48:24 +0100
Subject: [PATCH 6/9] ctdb-tools: Update error check for new string conversion
 wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 ctdb/tools/ctdb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c
index 8140d7337c5..98f1868e8eb 100644
--- a/ctdb/tools/ctdb.c
+++ b/ctdb/tools/ctdb.c
@@ -325,10 +325,9 @@ static bool parse_nodestring(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 		tok = strtok(ns, ",");
 		while (tok != NULL) {
 			uint32_t pnn;
-			char *endptr;
 
-			pnn = (uint32_t)strtoul_err(tok, &endptr, 0, &error);
-			if (error != 0 || (pnn == 0 && tok == endptr)) {
+			pnn = (uint32_t)strtoul_err(tok, NULL, 0, &error);
+			if (error != 0) {
 				fprintf(stderr, "Invalid node %s\n", tok);
 					return false;
 			}
-- 
2.20.1


From b917ea0c4c6643bfaaafec318c9bfaace05e193c Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 10:02:53 +0100
Subject: [PATCH 7/9] common-lib: Update error check for new string conversion
 wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 lib/ldb-samba/ldb_matching_rules.c | 14 ++------------
 lib/util/access.c                  |  2 +-
 lib/util/util_str.c                |  4 ++--
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/lib/ldb-samba/ldb_matching_rules.c b/lib/ldb-samba/ldb_matching_rules.c
index 7387c12f10d..0754e7e066b 100644
--- a/lib/ldb-samba/ldb_matching_rules.c
+++ b/lib/ldb-samba/ldb_matching_rules.c
@@ -393,12 +393,7 @@ static int dsdb_match_for_dns_to_tombstone_time(struct ldb_context *ldb,
 			return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
 		}
 		tombstone_time = strtoull_err(s, &p, 10, &error);
-		if (p == NULL ||
-		    p == s ||
-		    *p != '\0' ||
-		    error != 0 ||
-		    tombstone_time == ULLONG_MAX)
-		{
+		if (error != 0 || *p != '\0') {
 			DBG_ERR("Invalid timestamp string passed\n");
 			return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
 		}
@@ -529,12 +524,7 @@ static int dsdb_match_for_expunge(struct ldb_context *ldb,
 			return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
 		}
 		tombstone_time = strtoull_err(s, &p, 10, &error);
-		if (p == NULL ||
-		    p == s ||
-		    *p != '\0' ||
-		    error != 0 ||
-		    tombstone_time == ULLONG_MAX)
-		{
+		if (error != 0 || *p != '\0') {
 			return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
 		}
 	}
diff --git a/lib/util/access.c b/lib/util/access.c
index a05a47c15b2..960fe4b066c 100644
--- a/lib/util/access.c
+++ b/lib/util/access.c
@@ -75,7 +75,7 @@ static bool masked_match(const char *tok, const char *slash, const char *s)
 		unsigned long val;
 
 		val = strtoul_err(slash+1, &endp, 0, &error);
-		if (slash+1 == endp || (endp && *endp != '\0') || error != 0) {
+		if (error != 0 || *endp != '\0') {
 			return false;
 		}
 		if (!make_netmask(&ss_mask, &ss_tok, val)) {
diff --git a/lib/util/util_str.c b/lib/util/util_str.c
index 447919b087b..29a44836bde 100644
--- a/lib/util/util_str.c
+++ b/lib/util/util_str.c
@@ -70,7 +70,7 @@ _PUBLIC_ bool conv_str_size_error(const char * str, uint64_t * val)
 	}
 
 	lval = strtoull_err(str, &end, 10, &error);
-	if (end == NULL || end == str || error != 0) {
+	if (error != 0) {
 		return false;
 	}
 
@@ -112,7 +112,7 @@ _PUBLIC_ bool conv_str_u64(const char * str, uint64_t * val)
 	}
 
 	lval = strtoull_err(str, &end, 10, &error);
-	if (end == NULL || *end != '\0' || end == str || error != 0) {
+	if (error != 0 || *end != '\0') {
 		return false;
 	}
 
-- 
2.20.1


From 71f010f1a16bf5a061dd23c056cc6840d783bbec Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 10:06:35 +0100
Subject: [PATCH 8/9] libcli: Update error check for new string conversion
 wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 libcli/security/dom_sid.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
index ca7fd874752..ac34a92f19c 100644
--- a/libcli/security/dom_sid.c
+++ b/libcli/security/dom_sid.c
@@ -149,7 +149,7 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout,
 	}
 
 	conv = strtoul_err(p, &q, 10, &error);
-	if (!q || (*q != '-') || conv > UINT8_MAX || error != 0) {
+	if (error != 0 || (*q != '-') || conv > UINT8_MAX) {
 		goto format_error;
 	}
 	sidout->sid_rev_num = (uint8_t) conv;
@@ -161,7 +161,7 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout,
 
 	/* get identauth */
 	conv = strtoull_err(q, &q, 0, &error);
-	if (!q || conv & AUTHORITY_MASK || error != 0) {
+	if (conv & AUTHORITY_MASK || error != 0) {
 		goto format_error;
 	}
 
@@ -190,7 +190,7 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout,
 		}
 
 		conv = strtoull_err(q, &end, 10, &error);
-		if (end == q || conv > UINT32_MAX || error != 0) {
+		if (conv > UINT32_MAX || error != 0) {
 			goto format_error;
 		}
 
-- 
2.20.1


From 7af12532de52747d443b2572b47bb5f7eadf2d7d Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 10:11:39 +0100
Subject: [PATCH 9/9] source4: Update error check for new string conversion
 wrapper

The new string conversion wrappers detect and flag errors
which occured during the string to integer conversion.
Those modifications required an update of the callees
error checks.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source4/dsdb/samdb/ldb_modules/samldb.c | 2 +-
 source4/lib/socket/interface.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index ae99ebbe9ed..ea6e68b371a 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -3337,7 +3337,7 @@ static int verify_cidr(const char *cidr)
 		goto error;
 	}
 
-	if (*endptr != '\0' || endptr == slash + 1 || error != 0){
+	if (error != 0 || *endptr != '\0'){
 		DBG_INFO("CIDR mask is not a proper integer: %s\n", cidr);
 		goto error;
 	}
diff --git a/source4/lib/socket/interface.c b/source4/lib/socket/interface.c
index 494cbd4fdd2..9bf002c2f01 100644
--- a/source4/lib/socket/interface.c
+++ b/source4/lib/socket/interface.c
@@ -228,7 +228,7 @@ static void interpret_interface(TALLOC_CTX *mem_ctx,
 		int error = 0;
 
 		unsigned long val = strtoul_err(p, &endp, 0, &error);
-		if (p == endp || (endp && *endp != '\0') || error != 0) {
+		if (error != 0 || *endp != '\0') {
 			DEBUG(2,("interpret_interface: "
 				"can't determine netmask value from %s\n",
 				p));
-- 
2.20.1

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


More information about the samba-technical mailing list