Empy or false string conversion (was :fix string to integer conversion)
Christof Schmitt
cs at samba.org
Thu Apr 11 22:28:58 UTC 2019
Reviewed-by: Christof Schmitt <cs at samba.org>
Pushed to autobuild.
Christof
On Thu, Apr 11, 2019 at 09:19:42PM +0200, swen wrote:
> Hi Christof
>
> as suggested and agreed on by all involved parties off-list.
> Please find attached the patch-set with the split patch as
> suggested by you.
>
> Please review and push if, happy.
>
> Cheers Swen
> From 35ebf30506906509c6277f7fc129c14cb3bc9916 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 EINVAL if no conversion occured.
>
> Signed-off-by: Swen Schillig <swen at linux.ibm.com>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> lib/util/util.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/lib/util/util.c b/lib/util/util.c
> index b8eed3ca28c..83af14cac1e 100644
> --- a/lib/util/util.c
> +++ b/lib/util/util.c
> @@ -7,7 +7,8 @@
> Copyright (C) Jim McDonough (jmcd at us.ibm.com) 2003.
> Copyright (C) James J Myers 2003
> Copyright (C) Volker Lendecke 2010
> -
> + Copyright (C) Swen Schillig 2019
> +
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 3 of the License, or
> @@ -60,6 +61,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 +86,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 +116,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 +141,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 92e71ec838c880fe4cd50e56ca46030f3a986f4c 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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> source3/lib/interface.c | 2 +-
> source3/lib/util_str.c | 2 +-
> 2 files changed, 2 insertions(+), 2 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/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 2f84415036f0b6b949638b0096e877273c456cb3 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 03/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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> 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 42ef1d74ac18d87e279e1ede4d368511b48c16f1 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 04/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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> 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 fac4aa698de..39b8ccb097a 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 68646bbcdc9c59ee59b45e50aecadc81cca50abf 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 05/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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> 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 f97c64665df..d120213b70a 100644
> --- a/ctdb/protocol/protocol_util.c
> +++ b/ctdb/protocol/protocol_util.c
> @@ -291,7 +291,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;
> }
> @@ -330,7 +330,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 4d966e2a99831a8afb591d603726731b1d00c499 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 06/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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> 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 6b82bf779a9..8db0ec8e766 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 ea9b689ee3bb1b387970b5b3e0c1e662dd6fac0b 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 07/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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> 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 a61a07b0dc238dd82dc4b3376fc1490cf1cfb604 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 08/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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> 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 2d37c52afc89071caacb15ce424bc33ba9f974b1 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 09/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>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> 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 a8ea6c88459..4c773da5b19 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
>
>
> From 078f9a657de4179fdc10d76374b3db113c060672 Mon Sep 17 00:00:00 2001
> From: Swen Schillig <swen at linux.ibm.com>
> Date: Thu, 11 Apr 2019 09:52:05 +0200
> Subject: [PATCH 10/10] lib: remove duplicate check
>
> This check was supposed to be removed by c9f4b92a613.
>
> Signed-off-by: Swen Schillig <swen at linux.ibm.com>
> Reviewed-by: Ralph Boehme <slow at samba.org>
> ---
> source3/lib/messages_dgm.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
> index d73a6ad6a7c..5a79a234271 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;
> }
> --
> 2.20.1
>
More information about the samba-technical
mailing list