[PATCH] Fix string to integer conversion
swen
swen at linux.ibm.com
Tue Feb 5 13:02:03 UTC 2019
Hi Jeremy
On Mon, 2019-01-28 at 15:19 -0800, Jeremy Allison via samba-technical
wrote:
> On Mon, Jan 28, 2019 at 02:43:36PM +0100, swen wrote:
>
> > I still believe your suggested way is not good a good API.
> > Anyhow, because I believe that that solution is still better than
> > none,
> > I will prepare a patch with your code suggestion.
>
> Thanks Swen, but I'm out with the flu at the moment.
> I'm hoping to get better so I can go to FOSDEM to
> give my talk. I'll try and get to the patch as soon
> as health permits.
...as could be seen on samba-tech you're healthy again :-)
Please find attached the updated version of the
string conversion patch.
I fixed 2-3 typos and ran it on gitlab...(at some stage with success).
If time permits, please let me know if there's anything left to be
done.
Thanks for your support in advance.
Cheers Swen
-------------- next part --------------
From fe3441dc8692aa8e55c42d93ae7fe5621eca6845 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 09:42:13 +0100
Subject: [PATCH 01/19] util: Add two wrapper for string to int conversion
Adding wrapper strtoull_err and strtoul_err to handle
error conditions of the conversion process.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
lib/util/util.c | 28 ++++++++++++++++++++++++++++
lib/util/util.h | 7 +++++++
2 files changed, 35 insertions(+)
diff --git a/lib/util/util.c b/lib/util/util.c
index f52f69c6ef0..d2f7160e02e 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -47,6 +47,34 @@
* @brief Misc utility functions
*/
+unsigned long int
+strtoul_err(const char *nptr, char **endptr, int base, int *err)
+{
+ unsigned long int val;
+ int saved_errno = errno;
+
+ errno = 0;
+ val = strtoul(nptr, endptr, base);
+ *err = errno;
+
+ errno = saved_errno;
+ return val;
+}
+
+unsigned long long int
+strtoull_err(const char *nptr, char **endptr, int base, int *err)
+{
+ unsigned long long int val;
+ int saved_errno = errno;
+
+ errno = 0;
+ val = strtoull(nptr, endptr, base);
+ *err = errno;
+
+ errno = saved_errno;
+ return val;
+}
+
/**
Find a suitable temporary directory. The result should be copied immediately
as it may be overwritten by a subsequent call.
diff --git a/lib/util/util.h b/lib/util/util.h
index 5a0ce5cdb2a..fcd81759b9c 100644
--- a/lib/util/util.h
+++ b/lib/util/util.h
@@ -21,6 +21,13 @@
#ifndef __UTIL_SAMBA_UTIL_H__
#define __UTIL_SAMBA_UTIL_H__
+unsigned long int
+strtoul_err(const char *nptr, char **endptr, int base, int *err);
+
+unsigned long long int
+strtoull_err(const char *nptr, char **endptr, int base, int *err);
+
+
/**
* Write dump of binary data to a callback
*/
--
2.20.1
From c3a0d44f6268248c4edf635196cc28542435d01b Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 12:54:07 +0100
Subject: [PATCH 02/19] lib: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/lib/interface.c | 17 +++++++++++++----
source3/lib/messages_dgm.c | 17 +++++++++--------
source3/lib/namemap_cache.c | 17 +++++++----------
source3/lib/sysquotas.c | 7 ++++++-
source3/lib/tldap_util.c | 11 ++++++++++-
source3/lib/util_str.c | 5 +++--
source3/wscript_build | 2 ++
7 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/source3/lib/interface.c b/source3/lib/interface.c
index a3bc5d24e91..1d8dfd6501b 100644
--- a/source3/lib/interface.c
+++ b/source3/lib/interface.c
@@ -358,6 +358,7 @@ static void parse_extra_info(char *key, uint64_t *speed, uint32_t *cap,
while (key != NULL && *key != '\0') {
char *next_key;
char *val;
+ int error;
next_key = strchr_m(key, ',');
if (next_key != NULL) {
@@ -369,7 +370,10 @@ static void parse_extra_info(char *key, uint64_t *speed, uint32_t *cap,
*val++ = 0;
if (strequal_m(key, "speed")) {
- *speed = (uint64_t)strtoull(val, NULL, 0);
+ *speed = (uint64_t)strtoull_err(val, NULL, 0, &error);
+ if (error != 0) {
+ DBG_DEBUG("Invalid speed value (%s)\n", val);
+ }
} else if (strequal_m(key, "capability")) {
if (strequal_m(val, "RSS")) {
*cap |= FSCTL_NET_IFACE_RSS_CAPABLE;
@@ -380,7 +384,10 @@ static void parse_extra_info(char *key, uint64_t *speed, uint32_t *cap,
"'%s'\n", val);
}
} else if (strequal_m(key, "if_index")) {
- *if_index = (uint32_t)strtoul(val, NULL, 0);
+ *if_index = (uint32_t)strtoul_err(val, NULL, 0, &error);
+ if (error != 0) {
+ DBG_DEBUG("Invalid key value (%s)\n", val);
+ }
} else {
DBG_DEBUG("Key unknown: '%s'\n", key);
}
@@ -515,9 +522,11 @@ static void interpret_interface(char *token)
return;
}
} else {
+ int error = 0;
char *endp = NULL;
- unsigned long val = strtoul(p, &endp, 0);
- if (p == endp || (endp && *endp != '\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/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index 37eefeb0a4a..4836697bcf3 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -18,6 +18,7 @@
*/
#include "replace.h"
+#include "util/util.h"
#include "system/network.h"
#include "system/filesys.h"
#include "system/dir.h"
@@ -1442,6 +1443,7 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
{
char buf[25];
ssize_t rw_ret;
+ int error = 0;
unsigned long long unique;
char *endptr;
@@ -1451,13 +1453,11 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
}
buf[rw_ret] = '\0';
- unique = strtoull(buf, &endptr, 10);
- if ((unique == 0) && (errno == EINVAL)) {
- return EINVAL;
- }
- if ((unique == ULLONG_MAX) && (errno == ERANGE)) {
- return ERANGE;
+ unique = strtoull_err(buf, &endptr, 10, &error);
+ if (error != 0) {
+ return error;
}
+
if (endptr[0] != '\n') {
return EINVAL;
}
@@ -1599,6 +1599,7 @@ int messaging_dgm_forall(int (*fn)(pid_t pid, void *private_data),
struct messaging_dgm_context *ctx = global_dgm_context;
DIR *msgdir;
struct dirent *dp;
+ int error = 0;
if (ctx == NULL) {
return ENOTCONN;
@@ -1621,8 +1622,8 @@ int messaging_dgm_forall(int (*fn)(pid_t pid, void *private_data),
unsigned long pid;
int ret;
- pid = strtoul(dp->d_name, NULL, 10);
- if (pid == 0) {
+ pid = strtoul_err(dp->d_name, NULL, 10, &error);
+ if ((pid == 0) || (error != 0)) {
/*
* . and .. and other malformed entries
*/
diff --git a/source3/lib/namemap_cache.c b/source3/lib/namemap_cache.c
index fa179517f9f..42656ede0b7 100644
--- a/source3/lib/namemap_cache.c
+++ b/source3/lib/namemap_cache.c
@@ -22,6 +22,7 @@
#include "source3/lib/gencache.h"
#include "lib/util/debug.h"
#include "lib/util/strv.h"
+#include "lib/util/util.h"
#include "lib/util/talloc_stack.h"
#include "lib/util/charset/charset.h"
#include "libcli/security/dom_sid.h"
@@ -105,6 +106,7 @@ static void namemap_cache_find_sid_parser(
const char *domain;
const char *name;
const char *typebuf;
+ int error = 0;
char *endptr;
unsigned long type;
@@ -123,11 +125,8 @@ static void namemap_cache_find_sid_parser(
return;
}
- type = strtoul(typebuf, &endptr, 10);
- if (*endptr != '\0') {
- return;
- }
- if ((type == ULONG_MAX) && (errno == ERANGE)) {
+ type = strtoul_err(typebuf, &endptr, 10, &error);
+ if ((*endptr != '\0') || (error != 0)) {
return;
}
@@ -253,6 +252,7 @@ static void namemap_cache_find_name_parser(
const char *sid_endptr;
const char *typebuf;
char *endptr;
+ int error = 0;
struct dom_sid sid;
unsigned long type;
bool ok;
@@ -276,11 +276,8 @@ static void namemap_cache_find_name_parser(
return;
}
- type = strtoul(typebuf, &endptr, 10);
- if (*endptr != '\0') {
- return;
- }
- if ((type == ULONG_MAX) && (errno == ERANGE)) {
+ type = strtoul_err(typebuf, &endptr, 10, &error);
+ if ((*endptr != '\0') || (error != 0)) {
return;
}
diff --git a/source3/lib/sysquotas.c b/source3/lib/sysquotas.c
index 9b2d37b8375..c89dc96d690 100644
--- a/source3/lib/sysquotas.c
+++ b/source3/lib/sysquotas.c
@@ -250,6 +250,7 @@ static int command_get_quota(const char *path, enum SMB_QUOTA_TYPE qtype, unid_t
char *p2;
char *syscmd = NULL;
int _id = -1;
+ int error = 0;
switch(qtype) {
case SMB_USER_QUOTA_TYPE:
@@ -282,7 +283,11 @@ 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 */
- dp->qflags = strtoul(line, &p2, 10);
+ dp->qflags = strtoul_err(line, &p2, 10, &error);
+ if (error != 0) {
+ goto invalid_param;
+ }
+
p = p2;
while (p && *p && isspace(*p)) {
p++;
diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c
index 508c6c02f80..3135a8c4396 100644
--- a/source3/lib/tldap_util.c
+++ b/source3/lib/tldap_util.c
@@ -389,13 +389,22 @@ bool tldap_pull_uint64(struct tldap_message *msg, const char *attr,
{
char *str;
uint64_t result;
+ int error = 0;
str = tldap_talloc_single_attribute(msg, attr, talloc_tos());
if (str == NULL) {
DEBUG(10, ("Could not find attribute %s\n", attr));
return false;
}
- result = strtoull(str, NULL, 10);
+
+ result = strtoull_err(str, NULL, 10, &error);
+ if (error != 0) {
+ DBG_DEBUG("Attribute conversion failed (%s)\n",
+ strerror(error));
+ 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..a773b9b68f1 100644
--- a/source3/lib/util_str.c
+++ b/source3/lib/util_str.c
@@ -851,14 +851,15 @@ uint64_t conv_str_size(const char * str)
{
uint64_t lval;
char * end;
+ int error;
if (str == NULL || *str == '\0') {
return 0;
}
- lval = strtoull(str, &end, 10 /* base */);
+ lval = strtoull_err(str, &end, 10, &error);
- if (end == NULL || end == str) {
+ if (end == NULL || end == str || error != 0) {
return 0;
}
diff --git a/source3/wscript_build b/source3/wscript_build
index f25a27ba3a8..2b5b3ca2c4f 100644
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -367,6 +367,7 @@ bld.SAMBA3_LIBRARY('messages_dgm',
PTHREADPOOL
msghdr
genrand
+ samba-util
''',
private_library=True)
@@ -1362,6 +1363,7 @@ bld.RECURSE('smbd/notifyd')
bld.RECURSE('rpcclient')
bld.RECURSE('utils')
bld.RECURSE('nmbd')
+bld.RECURSE('lib/util')
bld.ENFORCE_GROUP_ORDERING()
bld.CHECK_PROJECT_RULES()
--
2.20.1
From 8fe24b7be23bda4ece6d55b62aabc5e4e110f480 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 13:12:09 +0100
Subject: [PATCH 03/19] groupdb: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/groupdb/mapping.c | 11 ++++++++++-
source3/groupdb/mapping_tdb.c | 12 ++++++++----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/source3/groupdb/mapping.c b/source3/groupdb/mapping.c
index 43722e777d4..77eb0d6e5cd 100644
--- a/source3/groupdb/mapping.c
+++ b/source3/groupdb/mapping.c
@@ -208,6 +208,7 @@ int smb_create_group(const char *unix_group, gid_t *new_gid)
char *add_script = NULL;
int ret = -1;
int fd = 0;
+ int error = 0;
*new_gid = 0;
@@ -244,7 +245,15 @@ int smb_create_group(const char *unix_group, gid_t *new_gid)
nread = read(fd, output, sizeof(output)-1);
if (nread > 0) {
output[nread] = '\0';
- *new_gid = (gid_t)strtoul(output, NULL, 10);
+ *new_gid = (gid_t)strtoul_err(output,
+ NULL,
+ 10,
+ &error);
+ if (error != 0) {
+ *new_gid = 0;
+ close(fd);
+ return -1;
+ }
}
close(fd);
diff --git a/source3/groupdb/mapping_tdb.c b/source3/groupdb/mapping_tdb.c
index d6a06ef199b..c80ff1f859a 100644
--- a/source3/groupdb/mapping_tdb.c
+++ b/source3/groupdb/mapping_tdb.c
@@ -860,6 +860,7 @@ static int convert_ldb_record(TDB_CONTEXT *ltdb, TDB_DATA key,
char *q;
uint32_t num_mem = 0;
struct dom_sid *members = NULL;
+ int error = 0;
p = (uint8_t *)data.dptr;
if (data.dsize < 8) {
@@ -974,8 +975,8 @@ 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) {
- map->gid = strtoul(val, &q, 10);
- if (*q) {
+ map->gid = strtoul_err(val, &q, 10, &error);
+ if (*q || (error != 0)) {
errno = EIO;
goto failed;
}
@@ -985,8 +986,11 @@ static int convert_ldb_record(TDB_CONTEXT *ltdb, TDB_DATA key,
goto failed;
}
} else if (strcasecmp_m(name, "sidNameUse") == 0) {
- map->sid_name_use = strtoul(val, &q, 10);
- if (*q) {
+ map->sid_name_use = strtoul_err(val,
+ &q,
+ 10,
+ &error);
+ if (*q || (error != 0)) {
errno = EIO;
goto failed;
}
--
2.20.1
From 6c5bc97f04c6a890038c76471b9bd9b07bdec1f8 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 13:36:45 +0100
Subject: [PATCH 04/19] utils: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/utils/net_idmap.c | 9 +++------
source3/utils/net_registry.c | 14 ++++++++++++--
source3/utils/net_rpc_registry.c | 8 +++++++-
source3/utils/net_sam.c | 5 +++--
source3/utils/pdbedit.c | 12 +++++++++---
source3/utils/regedit_dialog.c | 5 +++--
6 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/source3/utils/net_idmap.c b/source3/utils/net_idmap.c
index b49d5f43381..f6c9f3a1a3d 100644
--- a/source3/utils/net_idmap.c
+++ b/source3/utils/net_idmap.c
@@ -628,8 +628,9 @@ static bool parse_uint32(const char *str, uint32_t *result)
{
unsigned long val;
char *endptr;
+ int error = 0;
- val = strtoul(str, &endptr, 10);
+ val = strtoul_err(str, &endptr, 10, &error);
if (str == endptr) {
return false;
@@ -637,11 +638,7 @@ static bool parse_uint32(const char *str, uint32_t *result)
if (*endptr != '\0') {
return false;
}
- if ((val == ULONG_MAX) && (errno == ERANGE)) {
- return false;
- }
- if ((val & UINT32_MAX) != val) {
- /* overflow */
+ if (error != 0) {
return false;
}
*result = val; /* Potential crop */
diff --git a/source3/utils/net_registry.c b/source3/utils/net_registry.c
index 01a36b20e7c..b7711873561 100644
--- a/source3/utils/net_registry.c
+++ b/source3/utils/net_registry.c
@@ -509,7 +509,13 @@ static int net_registry_setvalue(struct net_context *c, int argc,
}
if (strequal(argv[2], "dword")) {
- uint32_t v = strtoul(argv[3], NULL, 10);
+ int error = 0;
+ uint32_t v = strtoul_err(argv[3], NULL, 10, &error);
+
+ if (error != 0) {
+ goto done;
+ }
+
value.type = REG_DWORD;
value.data = data_blob_talloc(ctx, NULL, 4);
SIVAL(value.data.data, 0, v);
@@ -641,7 +647,11 @@ static int net_registry_increment(struct net_context *c, int argc,
state.increment = 1;
if (argc == 3) {
- state.increment = strtoul(argv[2], NULL, 10);
+ int error = 0;
+ state.increment = strtoul_err(argv[2], NULL, 10, &error);
+ if (error != 0) {
+ 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..dc51137e15b 100644
--- a/source3/utils/net_rpc_registry.c
+++ b/source3/utils/net_rpc_registry.c
@@ -603,7 +603,13 @@ static NTSTATUS rpc_registry_setvalue_internal(struct net_context *c,
}
if (strequal(argv[2], "dword")) {
- uint32_t v = strtoul(argv[3], NULL, 10);
+ int error = 0;
+ uint32_t v = strtoul_err(argv[3], NULL, 10, &error);
+
+ if (error != 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..fc2a7baacef 100644
--- a/source3/utils/net_sam.c
+++ b/source3/utils/net_sam.c
@@ -484,6 +484,7 @@ static int net_sam_policy_set(struct net_context *c, int argc, const char **argv
uint32_t old_value = 0;
enum pdb_policy_type field;
char *endptr;
+ int err = 0;
if (argc != 2 || c->display_usage) {
d_fprintf(stderr, "%s\n%s",
@@ -500,9 +501,9 @@ static int net_sam_policy_set(struct net_context *c, int argc, const char **argv
value = -1;
}
else {
- value = strtoul(argv[1], &endptr, 10);
+ value = strtoul_err(argv[1], &endptr, 10, &err);
- if ((endptr == argv[1]) || (endptr[0] != '\0')) {
+ if ((endptr == argv[1]) || (endptr[0] != '\0') || (err != 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..51db6710ff3 100644
--- a/source3/utils/pdbedit.c
+++ b/source3/utils/pdbedit.c
@@ -598,9 +598,15 @@ 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);
-
- if ((endptr == kickoff_time) || (endptr[0] != '\0')) {
+ int error = 0;
+ uint32_t num = strtoul_err(kickoff_time,
+ &endptr,
+ 10,
+ &error);
+
+ if ((endptr == kickoff_time) ||
+ (endptr[0] != '\0') ||
+ (error != 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..aeea70ac22e 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;
+ int error = 0;
struct dialog_section_text_field *text_field =
talloc_get_type_abort(section, struct dialog_section_text_field);
@@ -1041,9 +1042,9 @@ bool dialog_section_text_field_get_uint(struct dialog_section *section,
if (buf == NULL) {
return false;
}
- *out = strtoull(buf, &endp, 0);
+ *out = strtoull_err(buf, &endp, 0, &error);
rv = true;
- if (endp == buf || endp == NULL || endp[0] != '\0') {
+ if (endp == buf || endp == NULL || endp[0] != '\0' || error != 0) {
rv = false;
}
--
2.20.1
From 77acd059051a50ccffbe1d7e6b9d78db5f5bb49f Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 13:57:15 +0100
Subject: [PATCH 05/19] passdb: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/passdb/account_pol.c | 7 +++-
source3/passdb/pdb_ldap.c | 72 +++++++++++++++++++++++++++++-------
source3/passdb/pdb_tdb.c | 6 ++-
3 files changed, 70 insertions(+), 15 deletions(-)
diff --git a/source3/passdb/account_pol.c b/source3/passdb/account_pol.c
index e566eca78eb..b1463c647b7 100644
--- a/source3/passdb/account_pol.c
+++ b/source3/passdb/account_pol.c
@@ -456,7 +456,12 @@ 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);
+ int error = 0;
+ uint32_t tmp = strtoul_err(cache_value, NULL, 10, &error);
+
+ if (error != 0) {
+ goto done;
+ }
*value = tmp;
ret = True;
}
diff --git a/source3/passdb/pdb_ldap.c b/source3/passdb/pdb_ldap.c
index 7f8903ba96d..273baac8c7f 100644
--- a/source3/passdb/pdb_ldap.c
+++ b/source3/passdb/pdb_ldap.c
@@ -982,6 +982,7 @@ static bool init_sam_from_ldap(struct ldapsam_privates *ldap_state,
struct dom_sid mapped_gsid;
const struct dom_sid *primary_gsid;
struct unixid id;
+ int error = 0;
ZERO_STRUCT(unix_pw);
@@ -995,7 +996,11 @@ static bool init_sam_from_ldap(struct ldapsam_privates *ldap_state,
ctx);
if (temp) {
/* We've got a uid, feed the cache */
- unix_pw.pw_uid = strtoul(temp, NULL, 10);
+ unix_pw.pw_uid = strtoul_err(temp, NULL, 10, &error);
+ if (error != 0) {
+ DBG_ERR("Failed to convert UID\n");
+ goto fn_exit;
+ }
have_uid = true;
}
temp = smbldap_talloc_single_attribute(
@@ -1005,7 +1010,11 @@ static bool init_sam_from_ldap(struct ldapsam_privates *ldap_state,
ctx);
if (temp) {
/* We've got a uid, feed the cache */
- unix_pw.pw_gid = strtoul(temp, NULL, 10);
+ unix_pw.pw_gid = strtoul_err(temp, NULL, 10, &error);
+ if (error != 0) {
+ DBG_ERR("Failed to convert GID\n");
+ goto fn_exit;
+ }
have_gid = true;
}
unix_pw.pw_gecos = smbldap_talloc_single_attribute(
@@ -2879,6 +2888,7 @@ static NTSTATUS ldapsam_enum_group_memberships(struct pdb_methods *methods,
uint32_t num_gids;
char *gidstr;
gid_t primary_gid = -1;
+ int error = 0;
*pp_sids = NULL;
num_sids = 0;
@@ -2928,7 +2938,11 @@ static NTSTATUS ldapsam_enum_group_memberships(struct pdb_methods *methods,
ret = NT_STATUS_INTERNAL_DB_CORRUPTION;
goto done;
}
- primary_gid = strtoul(gidstr, NULL, 10);
+ primary_gid = strtoul_err(gidstr, NULL, 10, &error);
+ if (error != 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,10 +3010,11 @@ static NTSTATUS ldapsam_enum_group_memberships(struct pdb_methods *methods,
str, sizeof(str)-1))
continue;
- gid = strtoul(str, &end, 10);
+ gid = strtoul_err(str, &end, 10, &error);
- if (PTR_DIFF(end, str) != strlen(str))
+ if ((PTR_DIFF(end, str) != strlen(str)) || (error != 0)) {
goto done;
+ }
if (gid == primary_gid) {
sid_copy(&(*pp_sids)[0], &sid);
@@ -4924,6 +4939,7 @@ static NTSTATUS ldapsam_get_new_rid(struct ldapsam_privates *priv,
int rc;
uint32_t nextRid = 0;
const char *dn;
+ int error = 0;
TALLOC_CTX *mem_ctx;
@@ -4959,21 +4975,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);
+ uint32_t tmp = (uint32_t)strtoul_err(value, NULL, 10, &error);
+ if (error != 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);
+ uint32_t tmp = (uint32_t)strtoul_err(value, NULL, 10, &error);
+ if (error != 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);
+ uint32_t tmp = (uint32_t)strtoul_err(value, NULL, 10, &error);
+ if (error != 0) {
+ goto done;
+ }
+
nextRid = MAX(nextRid, tmp);
}
@@ -5043,6 +5071,7 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
struct ldapsam_privates *priv =
(struct ldapsam_privates *)methods->private_data;
char *filter;
+ int error = 0;
struct dom_sid_buf buf;
const char *attrs[] = { "sambaGroupType", "gidNumber", "uidNumber",
NULL };
@@ -5106,7 +5135,11 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
goto done;
}
- id->id = strtoul(gid_str, NULL, 10);
+ id->id = strtoul_err(gid_str, NULL, 10, &error);
+ if (error != 0) {
+ goto done;
+ }
+
id->type = ID_TYPE_GID;
ret = True;
goto done;
@@ -5122,9 +5155,12 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
goto done;
}
- id->id = strtoul(value, NULL, 10);
- id->type = ID_TYPE_UID;
+ id->id = strtoul_err(value, NULL, 10, &error);
+ if (error != 0) {
+ goto done;
+ }
+ id->type = ID_TYPE_UID;
ret = True;
done:
TALLOC_FREE(mem_ctx);
@@ -5665,6 +5701,7 @@ static NTSTATUS ldapsam_create_dom_group(struct pdb_methods *my_methods,
struct dom_sid_buf buf;
gid_t gid = -1;
int rc;
+ int error = 0;
groupname = escape_ldap_string(talloc_tos(), name);
filter = talloc_asprintf(tmp_ctx, "(&(cn=%s)(objectClass=%s))",
@@ -5709,7 +5746,11 @@ static NTSTATUS ldapsam_create_dom_group(struct pdb_methods *my_methods,
return NT_STATUS_INTERNAL_DB_CORRUPTION;
}
- gid = strtoul(tmp, NULL, 10);
+ gid = strtoul_err(tmp, NULL, 10, &error);
+ if (error != 0) {
+ DBG_ERR("Failed to convert gidNumber\n");
+ return NT_STATUS_UNSUCCESSFUL;
+ }
dn = smbldap_talloc_dn(tmp_ctx, priv2ld(ldap_state), entry);
if (!dn) {
@@ -5916,6 +5957,7 @@ static NTSTATUS ldapsam_change_groupmem(struct pdb_methods *my_methods,
struct dom_sid member_sid;
struct dom_sid_buf buf;
int rc;
+ int error = 0;
switch (modop) {
case LDAP_MOD_ADD:
@@ -5981,7 +6023,11 @@ static NTSTATUS ldapsam_change_groupmem(struct pdb_methods *my_methods,
return NT_STATUS_INTERNAL_DB_CORRUPTION;
}
- user_gid = strtoul(gidstr, NULL, 10);
+ user_gid = strtoul_err(gidstr, NULL, 10, &error);
+ if (error != 0) {
+ DBG_ERR("Failed to convert user gid\n");
+ return NT_STATUS_UNSUCCESSFUL;
+ }
if (!sid_to_gid(&group_sid, &group_gid)) {
DEBUG (0, ("ldapsam_change_groupmem: Unable to get group gid from SID!\n"));
diff --git a/source3/passdb/pdb_tdb.c b/source3/passdb/pdb_tdb.c
index 91735ff7084..067150334a3 100644
--- a/source3/passdb/pdb_tdb.c
+++ b/source3/passdb/pdb_tdb.c
@@ -1173,6 +1173,7 @@ static int tdbsam_collect_rids(struct db_record *rec, void *private_data)
private_data, struct tdbsam_search_state);
size_t prefixlen = strlen(RIDPREFIX);
uint32_t rid;
+ int error = 0;
TDB_DATA key;
key = dbwrap_record_get_key(rec);
@@ -1182,7 +1183,10 @@ static int tdbsam_collect_rids(struct db_record *rec, void *private_data)
return 0;
}
- rid = strtoul((char *)key.dptr+prefixlen, NULL, 16);
+ rid = strtoul_err((char *)key.dptr+prefixlen, NULL, 16, &error);
+ if (error != 0) {
+ return 0;
+ }
ADD_TO_LARGE_ARRAY(state, uint32_t, rid, &state->rids, &state->num_rids,
&state->array_size);
--
2.20.1
From 30806dd91ce678bbf41cf134fef2400664ecee5f Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 14:07:39 +0100
Subject: [PATCH 06/19] winbindd: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/winbindd/idmap_ldap.c | 17 ++++++++++++-----
source3/winbindd/winbindd_lookuprids.c | 6 ++++--
source3/winbindd/winbindd_util.c | 24 +++++++++++++++++++-----
3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/source3/winbindd/idmap_ldap.c b/source3/winbindd/idmap_ldap.c
index 17cc7404f12..2bc9c082f00 100644
--- a/source3/winbindd/idmap_ldap.c
+++ b/source3/winbindd/idmap_ldap.c
@@ -222,6 +222,7 @@ static NTSTATUS idmap_ldap_allocate_id_internal(struct idmap_domain *dom,
const char **attr_list;
const char *type;
struct idmap_ldap_context *ctx;
+ int error = 0;
/* Only do query if we are online */
if (idmap_is_offline()) {
@@ -299,7 +300,11 @@ static NTSTATUS idmap_ldap_allocate_id_internal(struct idmap_domain *dom,
goto done;
}
- xid->id = strtoul(id_str, NULL, 10);
+ xid->id = strtoul_err(id_str, NULL, 10, &error);
+ if (error != 0) {
+ ret = NT_STATUS_UNSUCCESSFUL;
+ goto done;
+ }
/* make sure we still have room to grow */
@@ -641,6 +646,7 @@ static NTSTATUS idmap_ldap_unixids_to_sids(struct idmap_domain *dom,
int count;
int rc;
int i;
+ int error = 0;
/* Only do query if we are online */
if (idmap_is_offline()) {
@@ -769,8 +775,8 @@ again:
continue;
}
- id = strtoul(tmp, NULL, 10);
- if (!idmap_unix_id_is_in_range(id, dom)) {
+ id = strtoul_err(tmp, NULL, 10, &error);
+ if (error != 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));
@@ -947,6 +953,7 @@ again:
struct dom_sid sid;
struct dom_sid_buf buf;
uint32_t id;
+ int error = 0;
if (i == 0) { /* first entry */
entry = ldap_first_entry(
@@ -1005,8 +1012,8 @@ again:
continue;
}
- id = strtoul(tmp, NULL, 10);
- if (!idmap_unix_id_is_in_range(id, dom)) {
+ id = strtoul_err(tmp, NULL, 10, &error);
+ if (error != 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..959a4a7db38 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;
- rids[i] = strtoul(p, &q, 10);
- if (*q != '\n') {
+ int error = 0;
+
+ rids[i] = strtoul_err(p, &q, 10, &error);
+ if (error != 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..91a2f6ef197 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -461,6 +461,7 @@ static void trustdom_list_done(struct tevent_req *req)
uint32_t trust_type;
uint32_t trust_attribs;
uint32_t trust_flags;
+ int error = 0;
DBG_DEBUG("parsing response line '%s'\n", p);
@@ -506,7 +507,11 @@ static void trustdom_list_done(struct tevent_req *req)
break;
}
- trust_flags = (uint32_t)strtoul(q, NULL, 10);
+ trust_flags = (uint32_t)strtoul_err(q, NULL, 10, &error);
+ if (error != 0) {
+ DBG_ERR("Failed to convert trust_flags\n");
+ break;
+ }
q = strtok(NULL, "\\");
if (q == NULL) {
@@ -514,7 +519,11 @@ static void trustdom_list_done(struct tevent_req *req)
break;
}
- trust_type = (uint32_t)strtoul(q, NULL, 10);
+ trust_type = (uint32_t)strtoul_err(q, NULL, 10, &error);
+ if (error != 0) {
+ DBG_ERR("Failed to convert trust_type\n");
+ break;
+ }
q = strtok(NULL, "\n");
if (q == NULL) {
@@ -522,7 +531,11 @@ static void trustdom_list_done(struct tevent_req *req)
break;
}
- trust_attribs = (uint32_t)strtoul(q, NULL, 10);
+ trust_attribs = (uint32_t)strtoul_err(q, NULL, 10, &error);
+ if (error != 0) {
+ DBG_ERR("Failed to convert trust_attribs\n");
+ break;
+ }
if (!within_forest) {
trust_flags &= ~NETR_TRUST_FLAG_IN_FOREST;
@@ -2142,6 +2155,7 @@ bool parse_xidlist(TALLOC_CTX *mem_ctx, const char *xidstr,
struct unixid xid;
unsigned long long id;
char *endp;
+ int error = 0;
switch (p[0]) {
case 'U':
@@ -2156,8 +2170,8 @@ bool parse_xidlist(TALLOC_CTX *mem_ctx, const char *xidstr,
p += 1;
- id = strtoull(p, &endp, 10);
- if ((id == ULLONG_MAX) && (errno == ERANGE)) {
+ id = strtoull_err(p, &endp, 10, &error);
+ if (error != 0) {
goto fail;
}
if (*endp != '\n') {
--
2.20.1
From 7f59f3a0faf7768871625f1fdb82d94d97f028a9 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 14:30:15 +0100
Subject: [PATCH 07/19] modules: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/modules/vfs_preopen.c | 6 +++++-
source3/modules/vfs_snapper.c | 5 +++--
source3/modules/vfs_unityed_media.c | 6 +++++-
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/source3/modules/vfs_preopen.c b/source3/modules/vfs_preopen.c
index 84b13556195..24d33fafacd 100644
--- a/source3/modules/vfs_preopen.c
+++ b/source3/modules/vfs_preopen.c
@@ -345,6 +345,7 @@ static bool preopen_parse_fname(const char *fname, unsigned long *pnum,
const char *p;
char *q = NULL;
unsigned long num;
+ int error = 0;
p = strrchr_m(fname, '/');
if (p == NULL) {
@@ -363,7 +364,10 @@ static bool preopen_parse_fname(const char *fname, unsigned long *pnum,
return false;
}
- num = strtoul(p, (char **)&q, 10);
+ num = strtoul_err(p, (char **)&q, 10, &error);
+ if (error != 0) {
+ return false;
+ }
if (num+1 < num) {
/* overflow */
diff --git a/source3/modules/vfs_snapper.c b/source3/modules/vfs_snapper.c
index 443a940b17a..0f52f9e71c5 100644
--- a/source3/modules/vfs_snapper.c
+++ b/source3/modules/vfs_snapper.c
@@ -1218,6 +1218,7 @@ static NTSTATUS snapper_snap_path_to_id(TALLOC_CTX *mem_ctx,
char *str_idx;
char *str_end;
uint32_t snap_id;
+ int error = 0;
path_dup = talloc_strdup(mem_ctx, snap_path);
if (path_dup == NULL) {
@@ -1250,8 +1251,8 @@ static NTSTATUS snapper_snap_path_to_id(TALLOC_CTX *mem_ctx,
}
str_idx++;
- snap_id = strtoul(str_idx, &str_end, 10);
- if (str_idx == str_end) {
+ snap_id = strtoul_err(str_idx, &str_end, 10, &error);
+ if (error != 0 || str_idx == str_end) {
talloc_free(path_dup);
return NT_STATUS_INVALID_PARAMETER;
}
diff --git a/source3/modules/vfs_unityed_media.c b/source3/modules/vfs_unityed_media.c
index 235adf523a3..fa200e64b61 100644
--- a/source3/modules/vfs_unityed_media.c
+++ b/source3/modules/vfs_unityed_media.c
@@ -103,6 +103,7 @@ static bool get_digit_group(const char *path, uintmax_t *digit)
char *endp = NULL;
codepoint_t cp;
size_t size;
+ int error;
DEBUG(10, ("get_digit_group entering with path '%s'\n",
path));
@@ -120,7 +121,10 @@ static bool get_digit_group(const char *path, uintmax_t *digit)
return false;
}
if ((size == 1) && (isdigit(cp))) {
- *digit = (uintmax_t)strtoul(p, &endp, 10);
+ *digit = (uintmax_t)strtoul_err(p, &endp, 10, &error);
+ if (error != 0) {
+ return false;
+ }
DEBUG(10, ("num_suffix = '%ju'\n",
*digit));
return true;
--
2.20.1
From dd8349c4ae9e4af032cfee39181a0d41a65a102e Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 28 Jan 2019 14:35:30 +0100
Subject: [PATCH 08/19] rpcclient: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/rpcclient/cmd_samr.c | 7 ++++++-
source3/rpcclient/cmd_spoolss.c | 8 +++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/source3/rpcclient/cmd_samr.c b/source3/rpcclient/cmd_samr.c
index 7e396f92a48..898ae6ad6fc 100644
--- a/source3/rpcclient/cmd_samr.c
+++ b/source3/rpcclient/cmd_samr.c
@@ -1406,13 +1406,18 @@ static NTSTATUS cmd_samr_delete_alias(struct rpc_pipe_client *cli,
uint32_t alias_rid;
uint32_t access_mask = MAXIMUM_ALLOWED_ACCESS;
struct dcerpc_binding_handle *b = cli->binding_handle;
+ int error = 0;
if (argc != 3) {
printf("Usage: %s builtin|domain [rid|name]\n", argv[0]);
return NT_STATUS_OK;
}
- alias_rid = strtoul(argv[2], NULL, 10);
+ alias_rid = strtoul_err(argv[2], NULL, 10, &error);
+ if (error != 0) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
/* Open SAMR handle */
diff --git a/source3/rpcclient/cmd_spoolss.c b/source3/rpcclient/cmd_spoolss.c
index 0a850ddc6aa..f6d631761b2 100644
--- a/source3/rpcclient/cmd_spoolss.c
+++ b/source3/rpcclient/cmd_spoolss.c
@@ -2642,6 +2642,7 @@ static WERROR cmd_spoolss_setprinterdata(struct rpc_pipe_client *cli,
union spoolss_PrinterData data;
DATA_BLOB blob;
struct dcerpc_binding_handle *b = cli->binding_handle;
+ int error = 0;
/* parse the command arguments */
if (argc < 5) {
@@ -2707,7 +2708,12 @@ static WERROR cmd_spoolss_setprinterdata(struct rpc_pipe_client *cli,
W_ERROR_HAVE_NO_MEMORY(data.string);
break;
case REG_DWORD:
- data.value = strtoul(argv[4], NULL, 10);
+ data.value = strtoul_err(argv[4], NULL, 10, &error);
+ if (error != 0) {
+ result = WERR_INVALID_PARAMETER;
+ goto done;
+ }
+
break;
case REG_BINARY:
data.binary = strhex_to_data_blob(mem_ctx, argv[4]);
--
2.20.1
From 8fde0768ecedb5c6e817de0c817797ebc6ae45f6 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Tue, 29 Jan 2019 13:03:20 +0100
Subject: [PATCH 09/19] ctdb-protocol: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/protocol/protocol_util.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/ctdb/protocol/protocol_util.c b/ctdb/protocol/protocol_util.c
index 75427e44f50..f22afea20df 100644
--- a/ctdb/protocol/protocol_util.c
+++ b/ctdb/protocol/protocol_util.c
@@ -26,6 +26,7 @@
#include "protocol.h"
#include "protocol_util.h"
+#include "lib/util/util.h"
static struct {
enum ctdb_runstate runstate;
@@ -286,8 +287,8 @@ int ctdb_sock_addr_from_string(const char *str,
return EINVAL;
}
- port = strtoul(p+1, &endp, 10);
- if (endp == p+1 || *endp != '\0') {
+ port = strtoul_err(p+1, &endp, 10, &ret);
+ if (endp == p+1 || *endp != '\0' || ret != 0) {
/* Empty string or trailing garbage */
return EINVAL;
}
@@ -309,7 +310,7 @@ int ctdb_sock_addr_mask_from_string(const char *str,
unsigned int m;
char *endp = NULL;
ssize_t len;
- bool ret;
+ int ret;
if (addr == NULL || mask == NULL) {
return EINVAL;
@@ -325,8 +326,8 @@ int ctdb_sock_addr_mask_from_string(const char *str,
return EINVAL;
}
- m = strtoul(p+1, &endp, 10);
- if (endp == p+1 || *endp != '\0') {
+ m = strtoul_err(p+1, &endp, 10, &ret);
+ if (endp == p+1 || *endp != '\0' || ret != 0) {
/* Empty string or trailing garbage */
return EINVAL;
}
--
2.20.1
From 5c24c1c00d5768866e3887a0546235125c61689e Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Tue, 29 Jan 2019 13:07:56 +0100
Subject: [PATCH 10/19] ctdb-server: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/server/ctdb_recovery_helper.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/ctdb/server/ctdb_recovery_helper.c b/ctdb/server/ctdb_recovery_helper.c
index 7fdcc2e5a29..627b3ebdca3 100644
--- a/ctdb/server/ctdb_recovery_helper.c
+++ b/ctdb/server/ctdb_recovery_helper.c
@@ -30,6 +30,7 @@
#include "lib/util/sys_rw.h"
#include "lib/util/time.h"
#include "lib/util/tevent_unix.h"
+#include "lib/util/util.h"
#include "protocol/protocol.h"
#include "protocol/protocol_api.h"
@@ -2750,7 +2751,11 @@ int main(int argc, char *argv[])
write_fd = atoi(argv[1]);
sockpath = argv[2];
- generation = (uint32_t)strtoul(argv[3], NULL, 0);
+ generation = (uint32_t)strtoul_err(argv[3], NULL, 0, &ret);
+ if (ret != 0) {
+ fprintf(stderr, "recovery: unable to initialize generation\n");
+ goto failed;
+ }
mem_ctx = talloc_new(NULL);
if (mem_ctx == NULL) {
--
2.20.1
From 618255b5e3a757ec803b77d6c764b9f2a01f8917 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Tue, 29 Jan 2019 13:27:28 +0100
Subject: [PATCH 11/19] ctdb-tools: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/tools/ctdb.c | 45 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c
index bef9c5f97fb..677551823a8 100644
--- a/ctdb/tools/ctdb.c
+++ b/ctdb/tools/ctdb.c
@@ -315,6 +315,7 @@ static bool parse_nodestring(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
goto done;
} else {
char *ns, *tok;
+ int error = 0;
ns = talloc_strdup(mem_ctx, nodestring);
if (ns == NULL) {
@@ -326,8 +327,8 @@ static bool parse_nodestring(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
uint32_t pnn;
char *endptr;
- pnn = (uint32_t)strtoul(tok, &endptr, 0);
- if (pnn == 0 && tok == endptr) {
+ pnn = (uint32_t)strtoul_err(tok, &endptr, 0, &error);
+ if (error != 0 || (pnn == 0 && tok == endptr)) {
fprintf(stderr, "Invalid node %s\n", tok);
return false;
}
@@ -544,7 +545,10 @@ static bool db_exists(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
}
if (strncmp(db_arg, "0x", 2) == 0) {
- id = strtoul(db_arg, NULL, 0);
+ id = strtoul_err(db_arg, NULL, 0, &ret);
+ if (ret != 0) {
+ return false;
+ }
for (i=0; i<dbmap->num; i++) {
if (id == dbmap->dbs[i].db_id) {
db = &dbmap->dbs[i];
@@ -1089,7 +1093,10 @@ static int control_setvar(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
}
tunable.name = argv[0];
- tunable.value = strtoul(argv[1], NULL, 0);
+ tunable.value = strtoul_err(argv[1], NULL, 0, &ret);
+ if (ret != 0) {
+ return ret;
+ }
ret = ctdb_ctrl_set_tunable(mem_ctx, ctdb->ev, ctdb->client,
ctdb->cmd_pnn, TIMEOUT(), &tunable);
@@ -1875,7 +1882,10 @@ static int control_process_exists(TALLOC_CTX *mem_ctx,
pid = atoi(argv[0]);
if (argc == 2) {
- srvid = strtoull(argv[1], NULL, 0);
+ srvid = strtoull_err(argv[1], NULL, 0, &ret);
+ if (ret != 0) {
+ return ret;
+ }
}
if (srvid == 0) {
@@ -2779,7 +2789,10 @@ static int control_ban(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
}
ban_state.pnn = ctdb->cmd_pnn;
- ban_state.time = strtoul(argv[0], NULL, 0);
+ ban_state.time = strtoul_err(argv[0], NULL, 0, &ret);
+ if (ret != 0) {
+ return ret;
+ }
if (ban_state.time == 0) {
fprintf(stderr, "Ban time cannot be zero\n");
@@ -3099,7 +3112,10 @@ static int control_gettickles(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
}
if (argc == 2) {
- port = strtoul(argv[1], NULL, 10);
+ port = strtoul_err(argv[1], NULL, 10, &ret);
+ if (ret != 0) {
+ return ret;
+ }
}
ret = ctdb_sock_addr_from_string(argv[0], &addr, false);
@@ -3804,8 +3820,8 @@ static int control_moveip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
return 1;
}
- pnn = strtoul(argv[1], NULL, 10);
- if (pnn == CTDB_UNKNOWN_PNN) {
+ pnn = strtoul_err(argv[1], NULL, 10, &ret);
+ if (pnn == CTDB_UNKNOWN_PNN || ret != 0) {
fprintf(stderr, "Invalid PNN %s\n", argv[1]);
return 1;
}
@@ -5257,7 +5273,10 @@ static int control_tstore(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
ZERO_STRUCT(header);
if (argc > 3) {
- header.rsn = (uint64_t)strtoull(argv[3], NULL, 0);
+ header.rsn = (uint64_t)strtoull_err(argv[3], NULL, 0, &ret);
+ if (ret != 0) {
+ return ret;
+ }
}
if (argc > 4) {
header.dmaster = (uint32_t)atol(argv[4]);
@@ -6269,7 +6288,11 @@ int main(int argc, const char *argv[])
ctdb_timeout = getenv("CTDB_TIMEOUT");
if (ctdb_timeout != NULL) {
- options.maxruntime = strtoul(ctdb_timeout, NULL, 0);
+ options.maxruntime = strtoul_err(ctdb_timeout, NULL, 0, &ret);
+ if (ret != 0) {
+ fprintf(stderr, "Invalid value CTDB_TIMEOUT\n");
+ exit(1);
+ }
} else {
options.maxruntime = 120;
}
--
2.20.1
From 4b041d4bc76734ed95f0bc3d1b1b1f77ac06e241 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Tue, 29 Jan 2019 14:21:25 +0100
Subject: [PATCH 12/19] libwbclient: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
nsswitch/libwbclient/wbc_idmap.c | 10 +++++----
nsswitch/libwbclient/wbc_sid.c | 36 ++++++++++++++++++--------------
nsswitch/libwbclient/wscript | 2 +-
3 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/nsswitch/libwbclient/wbc_idmap.c b/nsswitch/libwbclient/wbc_idmap.c
index f61efb92b8d..29500b61e6e 100644
--- a/nsswitch/libwbclient/wbc_idmap.c
+++ b/nsswitch/libwbclient/wbc_idmap.c
@@ -24,6 +24,7 @@
#include "replace.h"
#include "libwbclient.h"
#include "../winbind_client.h"
+#include "lib/util/util.h"
/* Convert a Windows SID to a Unix uid, allocating an uid if needed */
wbcErr wbcCtxSidToUid(struct wbcContext *ctx, const struct wbcDomainSid *sid,
@@ -374,26 +375,27 @@ wbcErr wbcCtxSidsToUnixIds(struct wbcContext *ctx,
for (i=0; i<num_sids; i++) {
struct wbcUnixId *id = &ids[i];
char *q;
+ int error = 0;
switch (p[0]) {
case 'U':
id->type = WBC_ID_TYPE_UID;
- id->id.uid = strtoul(p+1, &q, 10);
+ id->id.uid = strtoul_err(p+1, &q, 10, &error);
break;
case 'G':
id->type = WBC_ID_TYPE_GID;
- id->id.gid = strtoul(p+1, &q, 10);
+ id->id.gid = strtoul_err(p+1, &q, 10, &error);
break;
case 'B':
id->type = WBC_ID_TYPE_BOTH;
- id->id.uid = strtoul(p+1, &q, 10);
+ id->id.uid = strtoul_err(p+1, &q, 10, &error);
break;
default:
id->type = WBC_ID_TYPE_NOT_SPECIFIED;
q = strchr(p, '\n');
break;
};
- if (q == NULL || q[0] != '\n') {
+ if (q == NULL || q[0] != '\n' || error != 0) {
goto wbc_err_invalid;
}
p = q+1;
diff --git a/nsswitch/libwbclient/wbc_sid.c b/nsswitch/libwbclient/wbc_sid.c
index 77445afc5e9..464f1aedd76 100644
--- a/nsswitch/libwbclient/wbc_sid.c
+++ b/nsswitch/libwbclient/wbc_sid.c
@@ -26,6 +26,7 @@
#include "replace.h"
#include "libwbclient.h"
#include "../winbind_client.h"
+#include "lib/util/util.h"
/* Convert a sid to a string into a buffer. Return the string
* length. If buflen is too small, return the string length that would
@@ -99,6 +100,7 @@ wbcErr wbcStringToSid(const char *str,
{
const char *p;
char *q;
+ int error;
uint64_t x;
wbcErr wbc_status = WBC_ERR_UNKNOWN_FAILURE;
@@ -120,8 +122,8 @@ wbcErr wbcStringToSid(const char *str,
/* Get the SID revision number */
p = str+2;
- x = (uint64_t)strtoul(p, &q, 10);
- if (x==0 || x > UINT8_MAX || !q || *q!='-') {
+ x = (uint64_t)strtoul_err(p, &q, 10, &error);
+ if (x == 0 || x > UINT8_MAX || !q || *q != '-' || error != 0) {
wbc_status = WBC_ERR_INVALID_SID;
BAIL_ON_WBC_ERROR(wbc_status);
}
@@ -133,8 +135,8 @@ wbcErr wbcStringToSid(const char *str,
* be expressed as a hex value, according to MS-DTYP.
*/
p = q+1;
- x = strtoull(p, &q, 0);
- if (!q || *q!='-' || (x & AUTHORITY_MASK)) {
+ x = strtoull_err(p, &q, 0, &error);
+ if (!q || *q != '-' || (x & AUTHORITY_MASK) || error != 0) {
wbc_status = WBC_ERR_INVALID_SID;
BAIL_ON_WBC_ERROR(wbc_status);
}
@@ -149,10 +151,10 @@ wbcErr wbcStringToSid(const char *str,
p = q +1;
sid->num_auths = 0;
while (sid->num_auths < WBC_MAXSUBAUTHS) {
- x = strtoull(p, &q, 10);
+ x = strtoull_err(p, &q, 10, &error);
if (p == q)
break;
- if (x > UINT32_MAX) {
+ if (x > UINT32_MAX || error != 0) {
wbc_status = WBC_ERR_INVALID_SID;
BAIL_ON_WBC_ERROR(wbc_status);
}
@@ -337,6 +339,7 @@ wbcErr wbcCtxLookupSids(struct wbcContext *ctx,
char *sidlist, *p, *q, *extra_data;
struct wbcDomainInfo *domains = NULL;
struct wbcTranslatedName *names = NULL;
+ int error;
buflen = num_sids * (WBC_SID_STRING_BUFLEN + 1) + 1;
@@ -386,8 +389,8 @@ wbcErr wbcCtxLookupSids(struct wbcContext *ctx,
p = extra_data;
- num_domains = strtoul(p, &q, 10);
- if (*q != '\n') {
+ num_domains = strtoul_err(p, &q, 10, &error);
+ if (*q != '\n' || error != 0) {
goto wbc_err_invalid;
}
p = q+1;
@@ -426,8 +429,8 @@ wbcErr wbcCtxLookupSids(struct wbcContext *ctx,
p = q+1;
}
- num_names = strtoul(p, &q, 10);
- if (*q != '\n') {
+ num_names = strtoul_err(p, &q, 10, &error);
+ if (*q != '\n' || error != 0) {
goto wbc_err_invalid;
}
p = q+1;
@@ -446,8 +449,8 @@ wbcErr wbcCtxLookupSids(struct wbcContext *ctx,
for (i=0; i<num_names; i++) {
- names[i].domain_index = strtoul(p, &q, 10);
- if (names[i].domain_index < 0) {
+ names[i].domain_index = strtoul_err(p, &q, 10, &error);
+ if (names[i].domain_index < 0 || error != 0) {
goto wbc_err_invalid;
}
if (names[i].domain_index >= num_domains) {
@@ -459,8 +462,8 @@ wbcErr wbcCtxLookupSids(struct wbcContext *ctx,
}
p = q+1;
- names[i].type = strtoul(p, &q, 10);
- if (*q != ' ') {
+ names[i].type = strtoul_err(p, &q, 10, &error);
+ if (*q != ' ' || error != 0) {
goto wbc_err_invalid;
}
p = q+1;
@@ -515,6 +518,7 @@ wbcErr wbcCtxLookupRids(struct wbcContext *ctx, struct wbcDomainSid *dom_sid,
size_t i, len, ridbuf_size;
char *ridlist;
char *p;
+ int error;
struct winbindd_request request;
struct winbindd_response response;
char *domain_name = NULL;
@@ -581,9 +585,9 @@ wbcErr wbcCtxLookupRids(struct wbcContext *ctx, struct wbcDomainSid *dom_sid,
goto done;
}
- types[i] = (enum wbcSidType)strtoul(p, &q, 10);
+ types[i] = (enum wbcSidType)strtoul_err(p, &q, 10, &error);
- if (*q != ' ') {
+ if (*q != ' ' || error != 0) {
wbc_status = WBC_ERR_INVALID_RESPONSE;
goto done;
}
diff --git a/nsswitch/libwbclient/wscript b/nsswitch/libwbclient/wscript
index 7bb612d670a..d926067df91 100644
--- a/nsswitch/libwbclient/wscript
+++ b/nsswitch/libwbclient/wscript
@@ -37,7 +37,7 @@ def build(bld):
wbc_pwd.c
wbc_sid.c
wbc_util.c''',
- deps='winbind-client',
+ deps='winbind-client samba-util',
pc_files='wbclient.pc',
public_headers='wbclient.h',
abi_directory='ABI',
--
2.20.1
From 9d544d8b071f86f9eb999db9cdbe5e394d5e5d24 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Tue, 29 Jan 2019 14:36:44 +0100
Subject: [PATCH 13/19] wbinfo: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
nsswitch/wbinfo.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/nsswitch/wbinfo.c b/nsswitch/wbinfo.c
index b6f0ff8ccbf..d67f7c75888 100644
--- a/nsswitch/wbinfo.c
+++ b/nsswitch/wbinfo.c
@@ -141,6 +141,7 @@ static bool parse_wbinfo_domain_user(const char *domuser, fstring domain,
static bool parse_mapping_arg(char *arg, int *id, char **sid)
{
char *tmp, *endptr;
+ int error;
if (!arg || !*arg)
return false;
@@ -153,9 +154,9 @@ static bool parse_mapping_arg(char *arg, int *id, char **sid)
/* Because atoi() can return 0 on invalid input, which would be a valid
* UID/GID we must use strtoul() and do error checking */
- *id = strtoul(tmp, &endptr, 10);
+ *id = strtoul_err(tmp, &endptr, 10, &error);
- if (endptr[0] != '\0')
+ if (endptr[0] != '\0' || error != 0)
return false;
return true;
@@ -1417,7 +1418,13 @@ static bool wbinfo_lookuprids(const char *domain, const char *arg)
p = arg;
while (next_token_talloc(mem_ctx, &p, &ridstr, " ,\n")) {
- uint32_t rid = strtoul(ridstr, NULL, 10);
+ int error;
+ uint32_t rid = strtoul_err(ridstr, NULL, 10, &error);
+
+ if (error != 0) {
+ d_printf("failed to conver rid\n");
+ goto done;
+ }
rids = talloc_realloc(mem_ctx, rids, uint32_t, num_rids + 1);
if (rids == NULL) {
d_printf("talloc_realloc failed\n");
--
2.20.1
From e068f51edacc76dd4f3ad7793076dfccc64b1d65 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 30 Jan 2019 08:33:02 +0100
Subject: [PATCH 14/19] common-lib: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
lib/ldb-samba/ldb_matching_rules.c | 19 +++++++++++++++----
lib/ldb-samba/ldif_handlers.c | 7 +++++--
lib/param/loadparm.c | 24 ++++++++++++++++++++----
lib/util/access.c | 6 ++++--
lib/util/asn1.c | 17 +++++++++++------
lib/util/util_str.c | 10 ++++++----
6 files changed, 61 insertions(+), 22 deletions(-)
diff --git a/lib/ldb-samba/ldb_matching_rules.c b/lib/ldb-samba/ldb_matching_rules.c
index 2aaaeb7450b..51909d966dc 100644
--- a/lib/ldb-samba/ldb_matching_rules.c
+++ b/lib/ldb-samba/ldb_matching_rules.c
@@ -383,15 +383,20 @@ static int dsdb_match_for_dns_to_tombstone_time(struct ldb_context *ldb,
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
} else {
char *p = NULL;
+ int error;
char s[value_to_match->length+1];
+
memcpy(s, value_to_match->data, value_to_match->length);
s[value_to_match->length] = 0;
if (s[0] == '\0' || s[0] == '-') {
DBG_ERR("Empty timestamp passed\n");
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
}
- tombstone_time = strtoull(s, &p, 10);
- if (p == NULL || p == s || *p != '\0' ||
+ tombstone_time = strtoull_err(s, &p, 10, &error);
+ if (p == NULL ||
+ p == s ||
+ *p != '\0' ||
+ error != 0 ||
tombstone_time == ULLONG_MAX) {
DBG_ERR("Invalid timestamp string passed\n");
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@@ -514,14 +519,20 @@ static int dsdb_match_for_expunge(struct ldb_context *ldb,
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
} else {
char *p = NULL;
+ int error;
char s[value_to_match->length+1];
+
memcpy(s, value_to_match->data, value_to_match->length);
s[value_to_match->length] = 0;
if (s[0] == '\0' || s[0] == '-') {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
}
- tombstone_time = strtoull(s, &p, 10);
- if (p == NULL || p == s || *p != '\0' || tombstone_time == ULLONG_MAX) {
+ tombstone_time = strtoull_err(s, &p, 10, &error);
+ if (p == NULL ||
+ p == s ||
+ *p != '\0' ||
+ error != 0 ||
+ tombstone_time == ULLONG_MAX) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
}
}
diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
index ecc02e51c1d..9ae82400c73 100644
--- a/lib/ldb-samba/ldif_handlers.c
+++ b/lib/ldb-samba/ldif_handlers.c
@@ -596,6 +596,8 @@ static int ldif_read_prefixMap(struct ldb_context *ldb, void *mem_ctx,
line = string;
while (line && line[0]) {
+ int error;
+
p=strchr(line, ';');
if (p) {
p[0] = '\0';
@@ -619,9 +621,10 @@ static int ldif_read_prefixMap(struct ldb_context *ldb, void *mem_ctx,
return -1;
}
- blob->ctr.dsdb.mappings[blob->ctr.dsdb.num_mappings].id_prefix = strtoul(line, &oid, 10);
+ blob->ctr.dsdb.mappings[blob->ctr.dsdb.num_mappings].id_prefix =
+ strtoul_err(line, &oid, 10, &error);
- if (oid[0] != ':') {
+ if (oid[0] != ':' || error != 0) {
talloc_free(tmp_ctx);
return -1;
}
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index f31ef2319ac..89849762760 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -331,13 +331,21 @@ int lp_int(const char *s)
*/
unsigned long lp_ulong(const char *s)
{
+ int error;
+ unsigned long int ret;
if (!s || !*s) {
- DEBUG(0,("lp_ulong(%s): is called with NULL!\n",s));
+ DBG_DEBUG("lp_ulong(%s): is called with NULL!\n",s);
return -1;
}
- return strtoul(s, NULL, 0);
+ ret = strtoul_err(s, NULL, 0, &error);
+ if (error != 0) {
+ DBG_DEBUG("lp_ulong(%s): conversion failed\n",s);
+ return -1;
+ }
+
+ return ret;
}
/**
@@ -345,13 +353,21 @@ unsigned long lp_ulong(const char *s)
*/
unsigned long long lp_ulonglong(const char *s)
{
+ int error;
+ unsigned long long int ret;
if (!s || !*s) {
- DEBUG(0, ("lp_ulonglong(%s): is called with NULL!\n", s));
+ DBG_DEBUG("lp_ulonglong(%s): is called with NULL!\n", s);
return -1;
}
- return strtoull(s, NULL, 0);
+ ret = strtoull_err(s, NULL, 0, &error);
+ if (error != 0) {
+ DBG_DEBUG("lp_ulonglong(%s): conversion failed\n",s);
+ return -1;
+ }
+
+ return ret;
}
/**
diff --git a/lib/util/access.c b/lib/util/access.c
index 7da0573a74d..7d52bc0278f 100644
--- a/lib/util/access.c
+++ b/lib/util/access.c
@@ -71,8 +71,10 @@ static bool masked_match(const char *tok, const char *slash, const char *s)
}
} else {
char *endp = NULL;
- unsigned long val = strtoul(slash+1, &endp, 0);
- if (slash+1 == endp || (endp && *endp != '\0')) {
+ int error;
+ unsigned long val = strtoul_err(slash+1, &endp, 0, &error);
+
+ if (slash+1 == endp || (endp && *endp != '\0') || error != 0) {
return false;
}
if (!make_netmask(&ss_mask, &ss_tok, val)) {
diff --git a/lib/util/asn1.c b/lib/util/asn1.c
index 60ddfa09bcf..736ca9cfe9f 100644
--- a/lib/util/asn1.c
+++ b/lib/util/asn1.c
@@ -273,15 +273,20 @@ bool ber_write_OID_String(TALLOC_CTX *mem_ctx, DATA_BLOB *blob, const char *OID)
const char *p = (const char *)OID;
char *newp;
int i;
+ int error;
if (!isdigit(*p)) return false;
- v = strtoul(p, &newp, 10);
- if (newp[0] != '.') return false;
+ v = strtoul_err(p, &newp, 10, &error);
+ if (newp[0] != '.' || error != 0) {
+ return false;
+ }
p = newp + 1;
if (!isdigit(*p)) return false;
- v2 = strtoul(p, &newp, 10);
- if (newp[0] != '.') return false;
+ v2 = strtoul_err(p, &newp, 10, &error);
+ if (newp[0] != '.' || error != 0) {
+ return false;
+ }
p = newp + 1;
/*the ber representation can't use more space than the string one */
@@ -293,8 +298,8 @@ bool ber_write_OID_String(TALLOC_CTX *mem_ctx, DATA_BLOB *blob, const char *OID)
i = 1;
while (*p) {
if (!isdigit(*p)) return false;
- v = strtoul(p, &newp, 10);
- if (newp[0] == '.') {
+ v = strtoul_err(p, &newp, 10, &error);
+ if (newp[0] == '.' || error != 0) {
p = newp + 1;
/* check for empty last component */
if (!*p) return false;
diff --git a/lib/util/util_str.c b/lib/util/util_str.c
index c7d91ca3744..7368f6f9c3f 100644
--- a/lib/util/util_str.c
+++ b/lib/util/util_str.c
@@ -63,13 +63,14 @@ _PUBLIC_ bool conv_str_size_error(const char * str, uint64_t * val)
{
char * end = NULL;
unsigned long long lval;
+ int error;
if (str == NULL || *str == '\0') {
return false;
}
- lval = strtoull(str, &end, 10 /* base */);
- if (end == NULL || end == str) {
+ lval = strtoull_err(str, &end, 10, &error);
+ if (end == NULL || end == str || error != 0) {
return false;
}
@@ -104,13 +105,14 @@ _PUBLIC_ bool conv_str_u64(const char * str, uint64_t * val)
{
char * end = NULL;
unsigned long long lval;
+ int error;
if (str == NULL || *str == '\0') {
return false;
}
- lval = strtoull(str, &end, 10 /* base */);
- if (end == NULL || *end != '\0' || end == str) {
+ lval = strtoull_err(str, &end, 10, &error);
+ if (end == NULL || *end != '\0' || end == str || error != 0) {
return false;
}
--
2.20.1
From 24fbd4b0c0c3981d917aab13c5366e48acab98e7 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 30 Jan 2019 08:39:15 +0100
Subject: [PATCH 15/19] libcli: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
libcli/security/dom_sid.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
index 2a0b60c3dd7..f36bd7016bc 100644
--- a/libcli/security/dom_sid.c
+++ b/libcli/security/dom_sid.c
@@ -24,6 +24,7 @@
#include "lib/util/data_blob.h"
#include "system/locale.h"
#include "lib/util/debug.h"
+#include "lib/util/util.h"
#include "librpc/gen_ndr/security.h"
#include "dom_sid.h"
@@ -132,6 +133,7 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout,
char *q;
/* BIG NOTE: this function only does SIDS where the identauth is not >= 2^32 */
uint64_t conv;
+ int error;
ZERO_STRUCTP(sidout);
@@ -146,8 +148,8 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout,
goto format_error;
}
- conv = strtoul(p, &q, 10);
- if (!q || (*q != '-') || conv > UINT8_MAX) {
+ conv = strtoul_err(p, &q, 10, &error);
+ if (!q || (*q != '-') || conv > UINT8_MAX || error != 0) {
goto format_error;
}
sidout->sid_rev_num = (uint8_t) conv;
@@ -158,8 +160,8 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout,
}
/* get identauth */
- conv = strtoull(q, &q, 0);
- if (!q || conv & AUTHORITY_MASK) {
+ conv = strtoull_err(q, &q, 0, &error);
+ if (!q || conv & AUTHORITY_MASK || error != 0) {
goto format_error;
}
@@ -187,8 +189,8 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout,
goto format_error;
}
- conv = strtoull(q, &end, 10);
- if (end == q || conv > UINT32_MAX) {
+ conv = strtoull_err(q, &end, 10, &error);
+ if (end == q || conv > UINT32_MAX || error != 0) {
goto format_error;
}
--
2.20.1
From f65ee583e5bd0887cea16188b9f74596077ffc13 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 30 Jan 2019 09:31:34 +0100
Subject: [PATCH 16/19] source4: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source4/dns_server/dlz_bind9.c | 6 +++++-
source4/dsdb/common/dsdb_dn.c | 5 +++--
source4/dsdb/common/util.c | 17 ++++++++++++----
source4/dsdb/samdb/ldb_modules/dirsync.c | 15 +++++++++++++-
.../samdb/ldb_modules/partition_metadata.c | 7 ++++++-
source4/dsdb/samdb/ldb_modules/samldb.c | 5 +++--
source4/dsdb/samdb/ldb_modules/schema_load.c | 8 +++++++-
source4/dsdb/schema/schema_prefixmap.c | 6 +++++-
source4/lib/registry/ldb.c | 20 +++++++++++++++++--
source4/lib/socket/interface.c | 6 ++++--
source4/libcli/resolve/dns_ex.c | 5 +++--
source4/nbt_server/wins/winsdb.c | 12 ++++++++++-
source4/rpc_server/lsa/dcesrv_lsa.c | 9 ++++++---
source4/torture/nbench/nbench.c | 8 +++++++-
source4/torture/smb2/sharemode.c | 7 ++++++-
source4/web_server/web_server.c | 7 ++++++-
16 files changed, 117 insertions(+), 26 deletions(-)
diff --git a/source4/dns_server/dlz_bind9.c b/source4/dns_server/dlz_bind9.c
index 82c72111a00..83f11aa6669 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;\
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..2ec02307e40 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;
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 d173a75ebb7..328750be81a 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -3842,6 +3842,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;
v = ldb_dn_get_extended_component(dn, component_name);
if (v == NULL) {
@@ -3856,7 +3857,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;
}
@@ -3875,6 +3879,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;
v = ldb_dn_get_extended_component(dn, component_name);
if (v == NULL) {
@@ -3888,7 +3893,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;
@@ -3942,6 +3950,7 @@ uint32_t dsdb_dn_val_rmd_flags(const struct ldb_val *val)
const char *p;
uint32_t flags;
char *end;
+ int error;
if (val->length < 13) {
return 0;
@@ -3950,8 +3959,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..5f60be14af7 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;
/* 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..ad145ea7847 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;
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..8615baff363 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;
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..46306820f16 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;
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..2be23087a12 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;
/* 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..e1ec4b31c31 100644
--- a/source4/lib/registry/ldb.c
+++ b/source4/lib/registry/ldb.c
@@ -75,8 +75,15 @@ static void reg_ldb_unpack_value(TALLOC_CTX *mem_ctx,
case REG_DWORD:
case REG_DWORD_BIG_ENDIAN:
if (val != NULL) {
+ int error;
/* The data is a plain DWORD */
- uint32_t tmp = strtoul((char *)val->data, NULL, 0);
+ uint32_t 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 +97,17 @@ static void reg_ldb_unpack_value(TALLOC_CTX *mem_ctx,
case REG_QWORD:
if (val != NULL) {
+ int error;
/* The data is a plain QWORD */
- uint64_t tmp = strtoull((char *)val->data, NULL, 0);
+ uint64_t 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..d6500874e0d 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;
+
+ 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..d76b0f819c7 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;
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..abd3c49bb50 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;
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..1d7410c2dfb 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;
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..e48997a779e 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;
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..8232e1e5af9 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;
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..93c147302ef 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;
+
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, ':');
--
2.20.1
From dad59e997e6f44d5c05b9d5498abc45e96b35a12 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 30 Jan 2019 10:23:14 +0100
Subject: [PATCH 17/19] third_party: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
third_party/nss_wrapper/nss_wrapper.c | 64 ++++++---------------
third_party/nss_wrapper/wscript | 2 +-
third_party/socket_wrapper/socket_wrapper.c | 11 +++-
third_party/socket_wrapper/wscript | 2 +-
third_party/uid_wrapper/uid_wrapper.c | 47 +++++++++++++--
third_party/uid_wrapper/wscript | 2 +-
6 files changed, 71 insertions(+), 57 deletions(-)
diff --git a/third_party/nss_wrapper/nss_wrapper.c b/third_party/nss_wrapper/nss_wrapper.c
index 81d900c3f42..d602a202a68 100644
--- a/third_party/nss_wrapper/nss_wrapper.c
+++ b/third_party/nss_wrapper/nss_wrapper.c
@@ -60,6 +60,7 @@
#include <search.h>
#include <assert.h>
+
/*
* Defining _POSIX_PTHREAD_SEMANTICS before including pwd.h and grp.h gives us
* the posix getpwnam_r(), getpwuid_r(), getgrnam_r and getgrgid_r calls on
@@ -138,6 +139,8 @@ typedef nss_status_t NSS_STATUS;
#define DNS_NAME_MAX 255
#endif
+#include "lib/util/util.h"
+
/* GCC have printf type attribute check. */
#ifdef HAVE_ATTRIBUTE_PRINTF_FORMAT
#define PRINTF_ATTRIBUTE(a,b) __attribute__ ((__format__ (__printf__, a, b)))
@@ -1566,6 +1569,7 @@ static void nwrap_init(void)
char *endptr;
size_t max_hostents_tmp;
int ok;
+ int error;
NWRAP_LOCK(nwrap_initialized);
if (nwrap_initialized) {
@@ -1588,9 +1592,11 @@ static void nwrap_init(void)
env = getenv("NSS_WRAPPER_MAX_HOSTENTS");
if (env != NULL) {
- max_hostents_tmp = (size_t)strtoul(env, &endptr, 10);
+ max_hostents_tmp =
+ (size_t)strtoul_err(env, &endptr, 10, &error);
if ((*env == '\0') ||
(*endptr != '\0') ||
+ (error != 0) ||
(max_hostents_tmp == 0)) {
NWRAP_LOG(NWRAP_LOG_DEBUG,
"Error parsing NSS_WRAPPER_MAX_HOSTENTS "
@@ -1883,6 +1889,7 @@ static bool nwrap_pw_parse_line(struct nwrap_cache *nwrap, char *line)
char *e;
struct passwd *pw;
size_t list_size;
+ int error;
nwrap_pw = (struct nwrap_pw *)nwrap->private_data;
@@ -1938,23 +1945,11 @@ static bool nwrap_pw_parse_line(struct nwrap_cache *nwrap, char *line)
*p = '\0';
p++;
e = NULL;
- pw->pw_uid = (uid_t)strtoul(c, &e, 10);
- if (c == e) {
+ pw->pw_uid = (uid_t)strtoul_err(c, &e, 10, &error);
+ if (c == e || e == NULL || e[0] != '\0' || error != 0) {
NWRAP_LOG(NWRAP_LOG_ERROR,
"Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
- return false;
- }
- if (e == NULL) {
- NWRAP_LOG(NWRAP_LOG_ERROR,
- "Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
- return false;
- }
- if (e[0] != '\0') {
- NWRAP_LOG(NWRAP_LOG_ERROR,
- "Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
+ line, c, strerror(error));
return false;
}
c = p;
@@ -1970,23 +1965,11 @@ static bool nwrap_pw_parse_line(struct nwrap_cache *nwrap, char *line)
*p = '\0';
p++;
e = NULL;
- pw->pw_gid = (gid_t)strtoul(c, &e, 10);
- if (c == e) {
- NWRAP_LOG(NWRAP_LOG_ERROR,
- "Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
- return false;
- }
- if (e == NULL) {
+ pw->pw_gid = (gid_t)strtoul_err(c, &e, 10, &error);
+ if (c == e || e == NULL || e[0] != '\0' || error != 0) {
NWRAP_LOG(NWRAP_LOG_ERROR,
"Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
- return false;
- }
- if (e[0] != '\0') {
- NWRAP_LOG(NWRAP_LOG_ERROR,
- "Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
+ line, c, strerror(error));
return false;
}
c = p;
@@ -2410,6 +2393,7 @@ static bool nwrap_gr_parse_line(struct nwrap_cache *nwrap, char *line)
struct group *gr;
size_t list_size;
unsigned nummem;
+ int error;
nwrap_gr = (struct nwrap_gr *)nwrap->private_data;
@@ -2460,23 +2444,11 @@ static bool nwrap_gr_parse_line(struct nwrap_cache *nwrap, char *line)
*p = '\0';
p++;
e = NULL;
- gr->gr_gid = (gid_t)strtoul(c, &e, 10);
- if (c == e) {
- NWRAP_LOG(NWRAP_LOG_ERROR,
- "Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
- return false;
- }
- if (e == NULL) {
- NWRAP_LOG(NWRAP_LOG_ERROR,
- "Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
- return false;
- }
- if (e[0] != '\0') {
+ gr->gr_gid = (gid_t)strtoul_err(c, &e, 10, &error);
+ if (c == e || e == NULL || e[0] != '\0' ||error != 0) {
NWRAP_LOG(NWRAP_LOG_ERROR,
"Invalid line[%s]: '%s' - %s",
- line, c, strerror(errno));
+ line, c, strerror(error));
return false;
}
c = p;
diff --git a/third_party/nss_wrapper/wscript b/third_party/nss_wrapper/wscript
index 2e9d1db13b1..3b64bebe36a 100644
--- a/third_party/nss_wrapper/wscript
+++ b/third_party/nss_wrapper/wscript
@@ -89,6 +89,6 @@ def build(bld):
# breaks preloading!
bld.SAMBA_LIBRARY('nss_wrapper',
source='nss_wrapper.c',
- deps='dl',
+ deps='dl samba-util',
install=False,
realname='libnss-wrapper.so')
diff --git a/third_party/socket_wrapper/socket_wrapper.c b/third_party/socket_wrapper/socket_wrapper.c
index df70df5008d..c41ec51f0ee 100644
--- a/third_party/socket_wrapper/socket_wrapper.c
+++ b/third_party/socket_wrapper/socket_wrapper.c
@@ -237,6 +237,12 @@ enum swrap_dbglvl_e {
* without changing the format above */
#define MAX_WRAPPED_INTERFACES 64
+#ifndef _PUBLIC_
+#define _PUBLIC_
+#endif
+
+#include "lib/util/util.h"
+
struct swrap_address {
socklen_t sa_socklen;
union {
@@ -1352,6 +1358,7 @@ static size_t socket_wrapper_max_sockets(void)
const char *s;
size_t tmp;
char *endp;
+ int error;
if (socket_info_max != 0) {
return socket_info_max;
@@ -1364,11 +1371,11 @@ static size_t socket_wrapper_max_sockets(void)
goto done;
}
- tmp = strtoul(s, &endp, 10);
+ tmp = strtoul_err(s, &endp, 10, &error);
if (s == endp) {
goto done;
}
- if (tmp == 0) {
+ if (tmp == 0 || error != 0) {
tmp = SOCKET_WRAPPER_MAX_SOCKETS_DEFAULT;
SWRAP_LOG(SWRAP_LOG_ERROR,
"Invalid number of sockets specified, "
diff --git a/third_party/socket_wrapper/wscript b/third_party/socket_wrapper/wscript
index 3cca13cc6fb..8f178ad8c40 100644
--- a/third_party/socket_wrapper/wscript
+++ b/third_party/socket_wrapper/wscript
@@ -85,6 +85,6 @@ def build(bld):
# breaks preloading!
bld.SAMBA_LIBRARY('socket_wrapper',
source='socket_wrapper.c',
- deps='dl pthread tirpc',
+ deps='dl pthread tirpc samba-util',
install=False,
realname='libsocket-wrapper.so')
diff --git a/third_party/uid_wrapper/uid_wrapper.c b/third_party/uid_wrapper/uid_wrapper.c
index 8f41ed92cb9..088b6190d0e 100644
--- a/third_party/uid_wrapper/uid_wrapper.c
+++ b/third_party/uid_wrapper/uid_wrapper.c
@@ -122,6 +122,16 @@
#define SAFE_FREE(x) do { if ((x) != NULL) {free(x); (x)=NULL;} } while(0)
#endif
+#ifndef _PUBLIC_
+#define _PUBLIC_
+#endif
+
+#ifndef uint8_t
+typedef unsigned char uint8_t;
+#endif
+
+#include "lib/util/util.h"
+
/*****************
* LOGGING
*****************/
@@ -980,46 +990,65 @@ static void uwrap_init_env(struct uwrap_thread *id)
{
const char *env;
int ngroups = 0;
+ int error;
env = getenv("UID_WRAPPER_INITIAL_RUID");
if (env != NULL && env[0] != '\0') {
UWRAP_LOG(UWRAP_LOG_DEBUG, "Initialize ruid with %s", env);
- id->ruid = strtoul(env, (char **)NULL, 10);
+ id->ruid = strtoul_err(env, (char **)NULL, 10, &error);
+ if (error != 0) {
+ goto error;
+ }
unsetenv("UID_WRAPPER_INITIAL_RUID");
}
env = getenv("UID_WRAPPER_INITIAL_EUID");
if (env != NULL && env[0] != '\0') {
UWRAP_LOG(UWRAP_LOG_DEBUG, "Initalize euid with %s", env);
- id->euid = strtoul(env, (char **)NULL, 10);
+ id->euid = strtoul_err(env, (char **)NULL, 10, &error);
+ if (error != 0) {
+ goto error;
+ }
unsetenv("UID_WRAPPER_INITIAL_EUID");
}
env = getenv("UID_WRAPPER_INITIAL_SUID");
if (env != NULL && env[0] != '\0') {
UWRAP_LOG(UWRAP_LOG_DEBUG, "Initalize suid with %s", env);
- id->suid = strtoul(env, (char **)NULL, 10);
+ id->suid = strtoul_err(env, (char **)NULL, 10, &error);
+ if (error != 0) {
+ goto error;
+ }
unsetenv("UID_WRAPPER_INITIAL_SUID");
}
env = getenv("UID_WRAPPER_INITIAL_RGID");
if (env != NULL && env[0] != '\0') {
UWRAP_LOG(UWRAP_LOG_DEBUG, "Initialize ruid with %s", env);
- id->rgid = strtoul(env, (char **)NULL, 10);
+ id->rgid = strtoul_err(env, (char **)NULL, 10, &error);
+ if (error != 0) {
+ goto error;
+ }
unsetenv("UID_WRAPPER_INITIAL_RGID");
}
env = getenv("UID_WRAPPER_INITIAL_EGID");
if (env != NULL && env[0] != '\0') {
UWRAP_LOG(UWRAP_LOG_DEBUG, "Initalize egid with %s", env);
- id->egid = strtoul(env, (char **)NULL, 10);
+ id->egid = strtoul_err(env, (char **)NULL, 10, &error);
+ if (error != 0) {
+ goto error;
+ }
unsetenv("UID_WRAPPER_INITIAL_EGID");
}
env = getenv("UID_WRAPPER_INITIAL_SGID");
if (env != NULL && env[0] != '\0') {
UWRAP_LOG(UWRAP_LOG_DEBUG, "Initalize sgid with %s", env);
- id->sgid = strtoul(env, (char **)NULL, 10);
+ id->sgid = strtoul_err(env, (char **)NULL, 10, &error);
+ if (error != 0) {
+ goto error;
+ }
unsetenv("UID_WRAPPER_INITIAL_SGID");
}
@@ -1090,6 +1119,12 @@ static void uwrap_init_env(struct uwrap_thread *id)
UWRAP_LOG(UWRAP_LOG_DEBUG, "Initalize groups with %s", env);
id->ngroups = ngroups;
}
+
+ return;
+
+error:
+ UWRAP_LOG(UWRAP_LOG_ERROR, "Init failed.");
+ exit(-1);
}
static void uwrap_init(void)
diff --git a/third_party/uid_wrapper/wscript b/third_party/uid_wrapper/wscript
index 61d8a189625..c7b2b003eaf 100644
--- a/third_party/uid_wrapper/wscript
+++ b/third_party/uid_wrapper/wscript
@@ -119,6 +119,6 @@ def build(bld):
# breaks preloading!
bld.SAMBA_LIBRARY('uid_wrapper',
source='uid_wrapper.c',
- deps='dl',
+ deps='dl samba-util',
install=False,
realname='libuid-wrapper.so')
--
2.20.1
From 9ecbd01ab1fc77b66f119050778379b13f3c0799 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 30 Jan 2019 10:28:52 +0100
Subject: [PATCH 18/19] ctdb-utils: Use wrapper for 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. This is achieved by using the
wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/utils/ceph/ctdb_mutex_ceph_rados_helper.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ctdb/utils/ceph/ctdb_mutex_ceph_rados_helper.c b/ctdb/utils/ceph/ctdb_mutex_ceph_rados_helper.c
index 7ef76c26e02..a7038f2dfce 100644
--- a/ctdb/utils/ceph/ctdb_mutex_ceph_rados_helper.c
+++ b/ctdb/utils/ceph/ctdb_mutex_ceph_rados_helper.c
@@ -301,10 +301,14 @@ int main(int argc, char *argv[])
cmr_state->pool_name = argv[3];
cmr_state->object = argv[4];
if (argc == 6) {
+ int error;
/* optional lock duration provided */
char *endptr = NULL;
- cmr_state->lock_duration_s = strtoull(argv[5], &endptr, 0);
- if ((endptr == argv[5]) || (*endptr != '\0')) {
+ cmr_state->lock_duration_s = strtoull_err(argv[5],
+ &endptr,
+ 0,
+ &error);
+ if ((endptr == argv[5]) || (*endptr != '\0') || (error != 0)) {
fprintf(stdout, CTDB_MUTEX_STATUS_ERROR);
ret = -EINVAL;
goto err_ctx_cleanup;
--
2.20.1
From 8d42054e3609ce111c5b5d684c9ffc56fe797c7a Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Tue, 5 Feb 2019 08:39:14 +0100
Subject: [PATCH 19/19] lib: modify string conversion wrapper to handle signed
input
The standard string conversion routines convert a "signed string"
into the positive representation of the resulting value.
This is not wanted and therefore now detected and flag'ed as an error.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
lib/util/util.c | 44 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 41 insertions(+), 3 deletions(-)
diff --git a/lib/util/util.c b/lib/util/util.c
index d2f7160e02e..428bad7d285 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -52,10 +52,29 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
{
unsigned long int val;
int saved_errno = errno;
+ char *needle = NULL;
+ char *tmp_endptr = NULL;
errno = 0;
- val = strtoul(nptr, endptr, base);
- *err = errno;
+ *err = 0;
+
+ val = strtoul(nptr, &tmp_endptr, base);
+
+ if (endptr != NULL) {
+ *endptr = tmp_endptr;
+ }
+
+ if (errno != 0) {
+ *err = errno;
+ errno = saved_errno;
+ return val;
+ }
+
+ /* did we convert a negative "number" ? */
+ needle = strchr(nptr, '-');
+ if (needle != NULL && needle < tmp_endptr) {
+ *err = EINVAL;
+ }
errno = saved_errno;
return val;
@@ -66,10 +85,29 @@ strtoull_err(const char *nptr, char **endptr, int base, int *err)
{
unsigned long long int val;
int saved_errno = errno;
+ char *needle = NULL;
+ char *tmp_endptr = NULL;
errno = 0;
+ *err = 0;
+
val = strtoull(nptr, endptr, base);
- *err = errno;
+
+ if (endptr != NULL) {
+ *endptr = tmp_endptr;
+ }
+
+ if (errno != 0) {
+ *err = errno;
+ errno = saved_errno;
+ return val;
+ }
+
+ /* did we convert a negative "number" ? */
+ needle = strchr(nptr, '-');
+ if (needle != NULL && needle < tmp_endptr) {
+ *err = EINVAL;
+ }
errno = saved_errno;
return val;
--
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/20190205/cff7a07e/signature.sig>
More information about the samba-technical
mailing list