[Fwd: Re: Empy or false string conversion (was :fix string to integer conversion)]
swen
swen at linux.ibm.com
Mon Mar 18 10:10:13 UTC 2019
Hi Ralph
Did you find some time to have a look ?
Cheers Swen
-------- Forwarded Message --------
From: swen <swen at linux.ibm.com>
Reply-To: swen at linux.ibm.com
To: Ralph Böhme <slow at samba.org>
Cc: samba-technical <samba-technical at lists.samba.org>, Jeremy Allison <
jra at samba.org>
Subject: Re: Empy or false string conversion (was :fix string to
integer conversion)
Date: Wed, 06 Mar 2019 11:39:53 +0100
Hi Ralph
On Mon, 2019-03-04 at 21:00 +0100, Ralph Böhme wrote:
> > Am 01.03.2019 um 08:32 schrieb swen <swen at linux.ibm.com>:
> >
> >
> > So could you please explain what is missing where under which
> > circumstances ?
>
> eg
>
> ctdb_sock_addr_from_string() currently does
>
> port = strtoul_err(p+1, &endp, 10, &ret);
> if (endp == p+1 || *endp != '\0' || ret != 0) {
> /* Empty string or trailing garbage */
> return EINVAL;
> }
>
> With your final nail in the coffin this should become just:
>
> port = strtoul_err(p+1, &endp, 10, &ret);
> if (ret != 0) {
> /* Empty string or trailing garbage */
> return EINVAL;
> }
>
Correct, it is not needed anymore, but it wouldn't do any harm or
prevent the acceptance of the addition itself, right ?
> Then we're at the point where we can rename the function and use a
> sane calling convention. :)
Right, I'd leave that to you then :-)
Anyway, I've attached a patchset dealing with what you've mentioned.
I hope we can finalize this patch series with the below.
Please review and push if happy.
..and thanks for your support in advance.
Cheers Swen
-------------- next part --------------
From 2eb2c024e63742d31dac4a34de3f017d9c50806e 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 01/10] 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..b0bfee978a3 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)
@@ -78,6 +79,13 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
*endptr = tmp_endptr;
}
+ /* got an invalid number-string resulting in no conversion */
+ if (nptr == tmp_endptr) {
+ *err = ENODATA;
+ errno = saved_errno;
+ return val;
+ }
+
if (errno != 0) {
*err = errno;
errno = saved_errno;
@@ -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)
@@ -125,6 +134,13 @@ strtoull_err(const char *nptr, char **endptr, int base, int *err)
*endptr = tmp_endptr;
}
+ /* got an invalid number-string resulting in no conversion */
+ if (nptr == tmp_endptr) {
+ *err = ENODATA;
+ errno = saved_errno;
+ return val;
+ }
+
if (errno != 0) {
*err = errno;
errno = saved_errno;
--
2.20.1
From e50f41033841d0529162fbf3304c053e8f13f32b 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 02/10] 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 | 7 -------
source3/lib/util_str.c | 2 +-
3 files changed, 2 insertions(+), 9 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..545166fb868 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -1470,17 +1470,10 @@ 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;
}
- 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 10b85c9415b86c7bdf0e46f8ee030b80bbbeb97b Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 6 Mar 2019 09:20:52 +0100
Subject: [PATCH 03/10] groupdb: 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/groupdb/mapping_tdb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/groupdb/mapping_tdb.c b/source3/groupdb/mapping_tdb.c
index c80ff1f859a..94c29912ce5 100644
--- a/source3/groupdb/mapping_tdb.c
+++ b/source3/groupdb/mapping_tdb.c
@@ -976,7 +976,7 @@ static int convert_ldb_record(TDB_CONTEXT *ltdb, TDB_DATA key,
* (objectclass, etc.) */
if (strcasecmp_m(name, "gidNumber") == 0) {
map->gid = strtoul_err(val, &q, 10, &error);
- if (*q || (error != 0)) {
+ if (error != 0) {
errno = EIO;
goto failed;
}
@@ -990,7 +990,7 @@ static int convert_ldb_record(TDB_CONTEXT *ltdb, TDB_DATA key,
&q,
10,
&error);
- if (*q || (error != 0)) {
+ if (error != 0) {
errno = EIO;
goto failed;
}
--
2.20.1
From 1ab353991d1e07b58a0a1060d837c10a8056eabf 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 04/10] 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 7b788f708980df5a2abc38a26d559886e99a0607 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 05/10] 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 8ff77ca09a43e39360ca9dc27f4642f9de86c235 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 06/10] 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 bfb835fca8db91addd0fc60b2de2d9641a81e6c8 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 07/10] 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 0d682a3a87b6a42dea323921ed88c13796059b06 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 08/10] 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..492a8e71773 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' || tombstone_time == ULLONG_MAX) {
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' || tombstone_time == ULLONG_MAX) {
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 d2418b30143af0d0fcb2a59edaef9f10e1392981 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 09/10] 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 7e12a8e385dd7fd136556feb262b0dc8933f6f09 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 10/10] 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/20190318/7d8acd31/signature.sig>
More information about the samba-technical
mailing list