[PATCH] Fix string to integer conversion
swen
swen at linux.ibm.com
Thu Feb 28 10:30:08 UTC 2019
Hi Ralph
On Thu, 2019-02-28 at 10:33 +0100, Ralph Böhme via samba-technical
wrote:
> Hi Swen,
>
> On Tue, Feb 26, 2019 at 04:45:37PM +0100, swen wrote:
> > The patchset contains now everything, I had to modify/fix and
> > FIXUPs suggestd by Ralph B.
> >
> > I hope we have it all now.
> >
> > The patchset is compile tested and currently running at gitlab.
> >
> > Thanks to all to help out here !
> >
> > Please review and piush if happy.
>
> thanks, much better!
ohhh noooo :-)
I knew this wouldn't be the end.
>
> There are still a few places where error is not initialized to 0. Can
> you check
> that?
>
> Furthermore, it seems in the commit
>
> passdb: Use wrapper for string to integer conversion
>
> the hunks changing source3/utils/net_registry.c and
> source3/utils/net_rpc_registry.c should have been applied to the
> previous
> commit.
>
> Can you fix that any submit an updated patchset? Please also rerun CI
> with the
> patchset and please include the link in the mail.
>
> Finally, I'm not sure we want the last patch being part of this
> patchset. Once
> strtoul_err() does that check, all callers must be updated to remove
> their own
> checks for the same error.
>
> Thanks for your patience with us nasty and grumpy reviewers! :)
Don't !
I'm really glad if things end up to everybody's satisfaction...and
finally in the code !
You really went through a lot of effort, thank you very much for that !
Hopefully I will be able to pay it back at some stage !
New patchset attached.
Running on CI at
https://gitlab.com/samba-team/devel/samba/commits/sswen-wrapper_for_string_conversion_v2
Cheers Swen
-------------- next part --------------
From 31768b3aef741e8a21f7d9d726465f9ee3499593 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/20] 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 | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
lib/util/util.h | 7 +++++++
2 files changed, 57 insertions(+)
diff --git a/lib/util/util.c b/lib/util/util.c
index f52f69c6ef0..855642c06bc 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -47,6 +47,56 @@
* @brief Misc utility functions
*/
+/**
+ * Convert a string to an unsigned long integer
+ *
+ * @param nptr pointer to string which is to be converted
+ * @param endptr [optional] reference to remainder of the string
+ * @param base base of the numbering scheme
+ * @param err error occured during conversion
+ * @result result of the conversion as provided by strtoul
+ *
+ * Currently the only errors detected are wrong base and a value overflow.
+ */
+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;
+}
+
+/**
+ * Convert a string to an unsigned long long integer
+ *
+ * @param nptr pointer to string which is to be converted
+ * @param endptr [optional] reference to remainder of the string
+ * @param base base of the numbering scheme
+ * @param err error occured during conversion
+ * @result result of the conversion as provided by strtoull
+ *
+ * Currently the only errors detected are wrong base and a value overflow.
+ */
+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 2b4a81d78cbce4e3c1f90309798c6769a18f43a1 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/20] lib: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/lib/interface.c | 18 ++++++++++++++----
source3/lib/messages_dgm.c | 15 ++++++++++-----
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, 52 insertions(+), 23 deletions(-)
diff --git a/source3/lib/interface.c b/source3/lib/interface.c
index a3bc5d24e91..342c92a61a2 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 = 0;
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,12 @@ 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;
+
+ 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..b3e99228442 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,15 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
}
buf[rw_ret] = '\0';
- unique = strtoull(buf, &endptr, 10);
+ unique = strtoull_err(buf, &endptr, 10, &error);
if ((unique == 0) && (errno == EINVAL)) {
return EINVAL;
}
- if ((unique == ULLONG_MAX) && (errno == ERANGE)) {
- return ERANGE;
+
+ if (error != 0) {
+ return error;
}
+
if (endptr[0] != '\n') {
return EINVAL;
}
@@ -1599,6 +1603,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 +1626,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..a0095d23978 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 = 0;
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 4fde8146070aaf58b9f493b2eb82de2adb469ec6 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/20] groupdb: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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 a60df8a5f690dd3d60b0136d97df7c3b1941ad9f 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/20] utils: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/utils/net_idmap.c | 9 +++------
source3/utils/net_registry.c | 16 ++++++++++++++--
source3/utils/net_rpc_registry.c | 9 ++++++++-
source3/utils/net_sam.c | 5 +++--
source3/utils/pdbedit.c | 11 ++++++++---
source3/utils/regedit_dialog.c | 5 +++--
6 files changed, 39 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..74224ddbf15 100644
--- a/source3/utils/net_registry.c
+++ b/source3/utils/net_registry.c
@@ -509,7 +509,14 @@ 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;
+
+ 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 +648,12 @@ 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..84936ee31ae 100644
--- a/source3/utils/net_rpc_registry.c
+++ b/source3/utils/net_rpc_registry.c
@@ -603,7 +603,14 @@ 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;
+
+ 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..c80d5411b00 100644
--- a/source3/utils/pdbedit.c
+++ b/source3/utils/pdbedit.c
@@ -598,9 +598,14 @@ 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;
+
+ 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 d8a8fa4094c6bbb4a02b347e0da5f94139620993 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/20] passdb: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/passdb/account_pol.c | 8 +++-
source3/passdb/pdb_ldap.c | 73 +++++++++++++++++++++++++++++-------
source3/passdb/pdb_tdb.c | 6 ++-
3 files changed, 72 insertions(+), 15 deletions(-)
diff --git a/source3/passdb/account_pol.c b/source3/passdb/account_pol.c
index e566eca78eb..4a180cb19bd 100644
--- a/source3/passdb/account_pol.c
+++ b/source3/passdb/account_pol.c
@@ -456,7 +456,13 @@ 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;
+
+ 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..85e9db8bb1f 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,8 @@ static NTSTATUS ldapsam_get_new_rid(struct ldapsam_privates *priv,
int rc;
uint32_t nextRid = 0;
const char *dn;
+ uint32_t tmp;
+ int error = 0;
TALLOC_CTX *mem_ctx;
@@ -4959,21 +4976,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);
+ 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);
+ 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);
+ tmp = (uint32_t)strtoul_err(value, NULL, 10, &error);
+ if (error != 0) {
+ goto done;
+ }
+
nextRid = MAX(nextRid, tmp);
}
@@ -5043,6 +5072,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 +5136,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 +5156,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 +5702,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 +5747,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 +5958,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 +6024,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 1e799fc936c47bc46cd9cab20581939281f84226 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/20] winbindd: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
source3/winbindd/idmap_ldap.c | 37 ++++++++++++++++++++------
source3/winbindd/winbindd_lookuprids.c | 6 +++--
source3/winbindd/winbindd_util.c | 24 +++++++++++++----
3 files changed, 52 insertions(+), 15 deletions(-)
diff --git a/source3/winbindd/idmap_ldap.c b/source3/winbindd/idmap_ldap.c
index 17cc7404f12..822abb9f559 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,16 +775,23 @@ again:
continue;
}
- id = strtoul(tmp, NULL, 10);
+ id = strtoul_err(tmp, NULL, 10, &error);
+ TALLOC_FREE(tmp);
+ if (error != 0) {
+ DEBUG(5, ("Requested id (%u) out of range (%u - %u). "
+ "Filtered!\n", id,
+ dom->low_id, dom->high_id));
+ TALLOC_FREE(sidstr);
+ continue;
+ }
+
if (!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));
TALLOC_FREE(sidstr);
- TALLOC_FREE(tmp);
continue;
}
- TALLOC_FREE(tmp);
map = idmap_find_map_by_id(&ids[bidx], type, id);
if (!map) {
@@ -947,6 +960,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,16 +1019,23 @@ again:
continue;
}
- id = strtoul(tmp, NULL, 10);
- if (!idmap_unix_id_is_in_range(id, dom)) {
+ id = strtoul_err(tmp, NULL, 10, &error);
+ TALLOC_FREE(tmp);
+ if (error != 0) {
+ DEBUG(5, ("Requested id (%u) out of range (%u - %u). "
+ "Filtered!\n", id,
+ dom->low_id, dom->high_id));
+ TALLOC_FREE(sidstr);
+ continue;
+ }
+
+ 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));
TALLOC_FREE(sidstr);
- TALLOC_FREE(tmp);
continue;
}
- TALLOC_FREE(tmp);
if (map->status == ID_MAPPED) {
DEBUG(1, ("WARNING: duplicate %s mapping in LDAP. "
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 0c040fe76687185a0995cc1213ebdf0de45985e7 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/20] modules: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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..06bbe99402c 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 = 0;
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 67c838d3db90ab30f9334601bc4ad3dc6d0af48f 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/20] rpcclient: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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 71a07a2165f93075356d597307eac7a9c8c6afe5 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/20] ctdb-protocol: Use wrapper for string to integer
conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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..99dbe82404d 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 = 0;
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 14463d358d4eb26305de0007e000426db15ca829 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/20] ctdb-server: Use wrapper for string to integer
conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/server/ctdb_recovery_helper.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/ctdb/server/ctdb_recovery_helper.c b/ctdb/server/ctdb_recovery_helper.c
index 7fdcc2e5a29..57e12b47037 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"
@@ -2739,7 +2740,7 @@ int main(int argc, char *argv[])
TALLOC_CTX *mem_ctx;
struct tevent_context *ev;
struct ctdb_client_context *client;
- int ret;
+ int ret = 0;
struct tevent_req *req;
uint32_t generation;
@@ -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 944b8dc19c340f823e530f807a67bca08ba8a9ce 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/20] ctdb-tools: Use wrapper for string to integer
conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/tools/ctdb.c | 66 +++++++++++++++++++++++++++++++++--------------
1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c
index bef9c5f97fb..8140d7337c5 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;
}
@@ -535,7 +536,8 @@ static bool db_exists(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
struct ctdb_dbid *db = NULL;
uint32_t id = 0;
const char *name = NULL;
- int ret, i;
+ int i;
+ int ret = 0;
ret = ctdb_ctrl_get_dbmap(mem_ctx, ctdb->ev, ctdb->client,
ctdb->pnn, TIMEOUT(), &dbmap);
@@ -544,7 +546,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];
@@ -1059,8 +1064,9 @@ static int control_setvar(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
{
struct ctdb_var_list *tun_var_list;
struct ctdb_tunable tunable;
- int ret, i;
bool found;
+ int i;
+ int ret = 0;
if (argc != 2) {
usage("setvar");
@@ -1089,7 +1095,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);
@@ -1867,7 +1876,8 @@ static int control_process_exists(TALLOC_CTX *mem_ctx,
{
pid_t pid;
uint64_t srvid = 0;
- int ret, status;
+ int status;
+ int ret = 0;
if (argc != 1 && argc != 2) {
usage("process-exists");
@@ -1875,7 +1885,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) {
@@ -2766,7 +2779,7 @@ static int control_ban(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
int argc, const char **argv)
{
struct ctdb_ban_state ban_state;
- int ret;
+ int ret = 0;
if (argc != 1) {
usage("ban");
@@ -2779,7 +2792,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");
@@ -3092,14 +3108,18 @@ static int control_gettickles(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
ctdb_sock_addr addr;
struct ctdb_tickle_list *tickles;
unsigned port = 0;
- int ret, i;
+ int i;
+ int ret = 0;
if (argc < 1 || argc > 2) {
usage("gettickles");
}
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);
@@ -3792,7 +3812,8 @@ static int control_moveip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
{
ctdb_sock_addr addr;
uint32_t pnn;
- int ret, retries = 0;
+ int retries = 0;
+ int ret = 0;
if (argc != 2) {
usage("moveip");
@@ -3804,8 +3825,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;
}
@@ -5228,7 +5249,7 @@ static int control_tstore(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
struct ctdb_ltdb_header header;
uint8_t header_buf[sizeof(struct ctdb_ltdb_header)];
size_t np;
- int ret;
+ int ret = 0;
if (argc < 3 || argc > 5) {
usage("tstore");
@@ -5257,7 +5278,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]);
@@ -6245,7 +6269,7 @@ int main(int argc, const char *argv[])
const struct ctdb_cmd *cmd;
int loglevel;
bool ok;
- int ret;
+ int ret = 0;
setlinebuf(stdout);
@@ -6269,7 +6293,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 3f106cdd902b4cfec3c3aa958e621b7210dfc7ee 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/20] libwbclient: Use wrapper for string to integer
conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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..514a7e8d738 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 = 0;
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 = 0;
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 = 0;
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 42aed2ba35ce325c631a1202f8476fa3e72122f7 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/20] wbinfo: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
nsswitch/wbinfo.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/nsswitch/wbinfo.c b/nsswitch/wbinfo.c
index b6f0ff8ccbf..8561a51e98a 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 = 0;
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,14 @@ 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 = 0;
+ uint32_t rid;
+
+ 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 e68f3563abb6b218f7727402d3282406f583a252 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/20] common-lib: Use wrapper for string to integer
conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
lib/ldb-samba/ldb_matching_rules.c | 23 ++++++++++++++++++-----
lib/ldb-samba/ldif_handlers.c | 7 +++++--
lib/param/loadparm.c | 24 ++++++++++++++++++++----
lib/util/access.c | 7 +++++--
lib/util/asn1.c | 17 +++++++++++------
lib/util/util_str.c | 10 ++++++----
6 files changed, 65 insertions(+), 23 deletions(-)
diff --git a/lib/ldb-samba/ldb_matching_rules.c b/lib/ldb-samba/ldb_matching_rules.c
index 2aaaeb7450b..7387c12f10d 100644
--- a/lib/ldb-samba/ldb_matching_rules.c
+++ b/lib/ldb-samba/ldb_matching_rules.c
@@ -383,16 +383,22 @@ 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 = 0;
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 == ULLONG_MAX) {
+ 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 +520,21 @@ static int dsdb_match_for_expunge(struct ldb_context *ldb,
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
} else {
char *p = NULL;
+ int error = 0;
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..d38cdd0c9a3 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 = 0;
+
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..f7f9fdc12fc 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 = 0;
+ 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 = 0;
+ 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..a05a47c15b2 100644
--- a/lib/util/access.c
+++ b/lib/util/access.c
@@ -71,8 +71,11 @@ 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 = 0;
+ unsigned long val;
+
+ 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..affa8f1df91 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 = 0;
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..447919b087b 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 = 0;
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 = 0;
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 069c607c47b9c3bcc468a81cd32465776559c2d4 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/20] libcli: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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..ca7fd874752 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 = 0;
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 1324aff94c39036e31b33628e90ea0d711a517a4 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/20] source4: Use wrapper for string to integer conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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..070990db3cf 100644
--- a/source4/dns_server/dlz_bind9.c
+++ b/source4/dns_server/dlz_bind9.c
@@ -307,8 +307,12 @@ static bool b9_dns_type(const char *type, enum dns_record_type *dtype)
#define DNS_PARSE_UINT(ret, str, sep, saveptr) do { \
char *istr = strtok_r(str, sep, &saveptr); \
+ int error = 0;\
if ((istr) == NULL) return false; \
- (ret) = strtoul(istr, NULL, 10); \
+ (ret) = strtoul_err(istr, NULL, 10, &error); \
+ if (error != 0) {\
+ return false;\
+ }\
} while (0)
/*
diff --git a/source4/dsdb/common/dsdb_dn.c b/source4/dsdb/common/dsdb_dn.c
index 900c6e2df63..2119a4faaf3 100644
--- a/source4/dsdb/common/dsdb_dn.c
+++ b/source4/dsdb/common/dsdb_dn.c
@@ -84,6 +84,7 @@ struct dsdb_dn *dsdb_dn_parse_trusted(TALLOC_CTX *mem_ctx, struct ldb_context *l
struct ldb_val bval;
struct ldb_val dval;
char *dn_str;
+ int error = 0;
enum dsdb_dn_format dn_format = dsdb_dn_oid_to_format(dn_oid);
@@ -134,8 +135,8 @@ struct dsdb_dn *dsdb_dn_parse_trusted(TALLOC_CTX *mem_ctx, struct ldb_context *l
}
errno = 0;
- blen = strtoul(p1, &p2, 10);
- if (errno != 0) {
+ blen = strtoul_err(p1, &p2, 10, &error);
+ if (error != 0) {
DEBUG(10, (__location__ ": failed\n"));
goto failed;
}
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index d173a75ebb7..36693c34f78 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 = 0;
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 = 0;
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 = 0;
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..291876e162b 100644
--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
@@ -157,11 +157,24 @@ static int dirsync_filter_entry(struct ldb_request *req,
for (i = msg->num_elements - 1; i >= 0; i--) {
attr = dsdb_attribute_by_lDAPDisplayName(dsc->schema, msg->elements[i].name);
if (ldb_attr_cmp(msg->elements[i].name, "uSNChanged") == 0) {
+ int error = 0;
/* Read the USN it will used at the end of the filtering
* to update the max USN in the cookie if we
* decide to keep this entry
*/
- val = strtoull((const char*)msg->elements[i].values[0].data, NULL, 0);
+ val = strtoull_err(
+ (const char*)msg->elements[i].values[0].data,
+ NULL,
+ 0,
+ &error);
+ if (error != 0) {
+ ldb_set_errstring(ldb,
+ "Failed to convert USN");
+ return ldb_module_done(dsc->req,
+ NULL,
+ NULL,
+ LDB_ERR_OPERATIONS_ERROR);
+ }
continue;
}
diff --git a/source4/dsdb/samdb/ldb_modules/partition_metadata.c b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
index 197e7b092e0..250fecbd9e6 100644
--- a/source4/dsdb/samdb/ldb_modules/partition_metadata.c
+++ b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
@@ -36,6 +36,7 @@ static int partition_metadata_get_uint64(struct ldb_module *module,
TDB_DATA tdb_key, tdb_data;
char *value_str;
TALLOC_CTX *tmp_ctx;
+ int error = 0;
data = talloc_get_type_abort(ldb_module_get_private(module),
struct partition_private_data);
@@ -73,7 +74,11 @@ static int partition_metadata_get_uint64(struct ldb_module *module,
return ldb_module_oom(module);
}
- *value = strtoull(value_str, NULL, 10);
+ *value = strtoull_err(value_str, NULL, 10, &error);
+ if (error != 0) {
+ return ldb_module_error(module, LDB_ERR_OPERATIONS_ERROR,
+ "partition_metadata: converision failed");
+ }
SAFE_FREE(tdb_data.dptr);
talloc_free(tmp_ctx);
diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index e69228c32c7..ae99ebbe9ed 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -3313,6 +3313,7 @@ static int verify_cidr(const char *cidr)
char *address_redux = NULL;
unsigned int address_len;
TALLOC_CTX *frame = NULL;
+ int error = 0;
DBG_DEBUG("CIDR is %s\n", cidr);
frame = talloc_stackframe();
@@ -3329,14 +3330,14 @@ static int verify_cidr(const char *cidr)
/* terminate the address for strchr, inet_pton */
*slash = '\0';
- mask = strtoul(slash + 1, &endptr, 10);
+ mask = strtoul_err(slash + 1, &endptr, 10, &error);
if (mask == 0){
DBG_INFO("Windows does not like the zero mask, "
"so nor do we: %s\n", cidr);
goto error;
}
- if (*endptr != '\0' || endptr == slash + 1){
+ if (*endptr != '\0' || endptr == slash + 1 || error != 0){
DBG_INFO("CIDR mask is not a proper integer: %s\n", cidr);
goto error;
}
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index 473a2e0a1f7..3b0781685cf 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -117,6 +117,7 @@ static int schema_metadata_get_uint64(struct schema_load_private_data *data,
char *value_str;
TALLOC_CTX *tmp_ctx;
int tdb_seqnum;
+ int error = 0;
if (!data) {
*value = default_value;
@@ -169,7 +170,12 @@ static int schema_metadata_get_uint64(struct schema_load_private_data *data,
* next time
*/
data->tdb_seqnum = tdb_seqnum;
- data->schema_seq_num_cache = strtoull(value_str, NULL, 10);
+ data->schema_seq_num_cache = strtoull_err(value_str, NULL, 10, &error);
+ if (error != 0) {
+ talloc_free(tmp_ctx);
+ return ldb_module_error(data->module, LDB_ERR_OPERATIONS_ERROR,
+ "Failed to convert value");
+ }
*value = data->schema_seq_num_cache;
diff --git a/source4/dsdb/schema/schema_prefixmap.c b/source4/dsdb/schema/schema_prefixmap.c
index c30aecc0ec2..482a35f2165 100644
--- a/source4/dsdb/schema/schema_prefixmap.c
+++ b/source4/dsdb/schema/schema_prefixmap.c
@@ -216,6 +216,7 @@ static WERROR _dsdb_pfm_make_binary_oid(const char *full_oid, TALLOC_CTX *mem_ct
{
uint32_t last_subid;
const char *oid_subid;
+ int error = 0;
/* make last sub-identifier value */
oid_subid = strrchr(full_oid, '.');
@@ -223,7 +224,10 @@ static WERROR _dsdb_pfm_make_binary_oid(const char *full_oid, TALLOC_CTX *mem_ct
return WERR_INVALID_PARAMETER;
}
oid_subid++;
- last_subid = strtoul(oid_subid, NULL, 10);
+ last_subid = strtoul_err(oid_subid, NULL, 10, &error);
+ if (error != 0) {
+ return WERR_INVALID_PARAMETER;
+ }
/* encode oid in BER format */
if (!ber_write_OID_String(mem_ctx, _bin_oid, full_oid)) {
diff --git a/source4/lib/registry/ldb.c b/source4/lib/registry/ldb.c
index 8bb6fd5c10f..ec293c46149 100644
--- a/source4/lib/registry/ldb.c
+++ b/source4/lib/registry/ldb.c
@@ -75,8 +75,16 @@ static void reg_ldb_unpack_value(TALLOC_CTX *mem_ctx,
case REG_DWORD:
case REG_DWORD_BIG_ENDIAN:
if (val != NULL) {
+ int error = 0;
/* The data is a plain DWORD */
- uint32_t tmp = strtoul((char *)val->data, NULL, 0);
+ uint32_t tmp;
+
+ tmp = strtoul_err((char *)val->data, NULL, 0, &error);
+ if (error != 0) {
+ data->data = NULL;
+ data->length = 0;
+ break;
+ }
data->data = talloc_size(mem_ctx, sizeof(uint32_t));
if (data->data != NULL) {
SIVAL(data->data, 0, tmp);
@@ -90,8 +98,16 @@ static void reg_ldb_unpack_value(TALLOC_CTX *mem_ctx,
case REG_QWORD:
if (val != NULL) {
+ int error = 0;
/* The data is a plain QWORD */
- uint64_t tmp = strtoull((char *)val->data, NULL, 0);
+ uint64_t tmp;
+
+ tmp = strtoull_err((char *)val->data, NULL, 0, &error);
+ if (error != 0) {
+ data->data = NULL;
+ data->length = 0;
+ break;
+ }
data->data = talloc_size(mem_ctx, sizeof(uint64_t));
if (data->data != NULL) {
SBVAL(data->data, 0, tmp);
diff --git a/source4/lib/socket/interface.c b/source4/lib/socket/interface.c
index 594df1a0526..494cbd4fdd2 100644
--- a/source4/lib/socket/interface.c
+++ b/source4/lib/socket/interface.c
@@ -225,8 +225,10 @@ static void interpret_interface(TALLOC_CTX *mem_ctx,
}
} else {
char *endp = NULL;
- unsigned long val = strtoul(p, &endp, 0);
- if (p == endp || (endp && *endp != '\0')) {
+ int error = 0;
+
+ unsigned long val = strtoul_err(p, &endp, 0, &error);
+ if (p == endp || (endp && *endp != '\0') || error != 0) {
DEBUG(2,("interpret_interface: "
"can't determine netmask value from %s\n",
p));
diff --git a/source4/libcli/resolve/dns_ex.c b/source4/libcli/resolve/dns_ex.c
index 6174b617cb3..a6863aed59e 100644
--- a/source4/libcli/resolve/dns_ex.c
+++ b/source4/libcli/resolve/dns_ex.c
@@ -514,6 +514,7 @@ static void pipe_handler(struct tevent_context *ev, struct tevent_fd *fde,
uint32_t port = 0;
char *p = strrchr(addrs[i], '@');
char *n;
+ int error = 0;
if (!p) {
composite_error(c, NT_STATUS_OBJECT_NAME_NOT_FOUND);
@@ -536,8 +537,8 @@ static void pipe_handler(struct tevent_context *ev, struct tevent_fd *fde,
composite_error(c, NT_STATUS_OBJECT_NAME_NOT_FOUND);
return;
}
- port = strtoul(p, NULL, 10);
- if (port > UINT16_MAX) {
+ port = strtoul_err(p, NULL, 10, &error);
+ if (port > UINT16_MAX || error != 0) {
composite_error(c, NT_STATUS_OBJECT_NAME_NOT_FOUND);
return;
}
diff --git a/source4/nbt_server/wins/winsdb.c b/source4/nbt_server/wins/winsdb.c
index fa9a6154b9a..5184d7de7b0 100644
--- a/source4/nbt_server/wins/winsdb.c
+++ b/source4/nbt_server/wins/winsdb.c
@@ -148,6 +148,7 @@ static NTSTATUS winsdb_nbt_name(TALLOC_CTX *mem_ctx, struct ldb_dn *dn, struct n
struct nbt_name *name;
unsigned int comp_num;
uint32_t cur = 0;
+ int error = 0;
name = talloc(mem_ctx, struct nbt_name);
if (!name) {
@@ -181,7 +182,16 @@ static NTSTATUS winsdb_nbt_name(TALLOC_CTX *mem_ctx, struct ldb_dn *dn, struct n
}
if (comp_num > cur && strcasecmp("type", ldb_dn_get_component_name(dn, cur)) == 0) {
- name->type = strtoul((char *)ldb_dn_get_component_val(dn, cur)->data, NULL, 0);
+ name->type =
+ strtoul_err(
+ (char *)ldb_dn_get_component_val(dn, cur)->data,
+ NULL,
+ 0,
+ &error);
+ if (error != 0) {
+ status = NT_STATUS_INTERNAL_DB_CORRUPTION;
+ goto failed;
+ }
cur++;
} else {
status = NT_STATUS_INTERNAL_DB_CORRUPTION;
diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index cbbd9f482f2..64d2ec9991a 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -1698,6 +1698,7 @@ static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx,
uint32_t orig_uint = 0;
unsigned int flags = 0;
int ret;
+ int error = 0;
orig_val = ldb_msg_find_ldb_val(orig, attribute);
if (!orig_val || !orig_val->data) {
@@ -1705,9 +1706,11 @@ static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx,
flags = LDB_FLAG_MOD_ADD;
} else {
- errno = 0;
- orig_uint = strtoul((const char *)orig_val->data, NULL, 0);
- if (errno != 0 || orig_uint != value) {
+ orig_uint = strtoul_err((const char *)orig_val->data,
+ NULL,
+ 0,
+ &error);
+ if (error != 0 || orig_uint != value) {
/* replace also if can't get value */
flags = LDB_FLAG_MOD_REPLACE;
}
diff --git a/source4/torture/nbench/nbench.c b/source4/torture/nbench/nbench.c
index 8e4a984c9b5..83395123c33 100644
--- a/source4/torture/nbench/nbench.c
+++ b/source4/torture/nbench/nbench.c
@@ -96,6 +96,8 @@ again:
while (fgets(line, sizeof(line)-1, f)) {
NTSTATUS status;
const char **params0, **params;
+ unsigned long int tmp;
+ int error = 0;
nbench_line_count++;
@@ -138,7 +140,11 @@ again:
/* accept numeric or string status codes */
if (strncmp(params[i-1], "0x", 2) == 0) {
- status = NT_STATUS(strtoul(params[i-1], NULL, 16));
+ tmp = strtoul_err(params[i-1], NULL, 16, &error);
+ if (error != 0) {
+ tmp = error;
+ }
+ status = NT_STATUS(tmp);
} else {
status = nt_status_string_to_code(params[i-1]);
}
diff --git a/source4/torture/smb2/sharemode.c b/source4/torture/smb2/sharemode.c
index 8c7403c8302..b76aa4ccd7d 100644
--- a/source4/torture/smb2/sharemode.c
+++ b/source4/torture/smb2/sharemode.c
@@ -186,12 +186,17 @@ bool torture_smb2_check_sharemode(struct torture_context *tctx)
struct smb2_create create = { };
NTSTATUS status;
bool ret = true;
+ int error = 0;
sharemode_string = torture_setting_string(tctx, "sharemode", "RWD");
sharemode = smb2_util_share_access(sharemode_string);
access_string = torture_setting_string(tctx, "access", "0xf01ff");
- access = strtoul(access_string, NULL, 0);
+ access = strtoul_err(access_string, NULL, 0, &error);
+ if (error != 0) {
+ torture_comment(tctx, "Initializing access failed.\n");
+ return false;
+ }
filename = torture_setting_string(tctx, "filename", "testfile");
operation = torture_setting_string(tctx, "operation", "WD");
diff --git a/source4/web_server/web_server.c b/source4/web_server/web_server.c
index 7c2717368e3..a1db34be21a 100644
--- a/source4/web_server/web_server.c
+++ b/source4/web_server/web_server.c
@@ -107,6 +107,8 @@ void websrv_output(struct websrv_context *web, const void *data, size_t length)
*/
NTSTATUS http_parse_header(struct websrv_context *web, const char *line)
{
+ int error = 0;
+
if (line[0] == 0) {
web->input.end_of_headers = true;
} else if (strncasecmp(line,"GET ", 4)==0) {
@@ -118,7 +120,10 @@ NTSTATUS http_parse_header(struct websrv_context *web, const char *line)
http_error(web, "400 Bad request", "This server only accepts GET and POST requests");
return NT_STATUS_INVALID_PARAMETER;
} else if (strncasecmp(line, "Content-Length: ", 16)==0) {
- web->input.content_length = strtoul(&line[16], NULL, 10);
+ web->input.content_length = strtoul_err(&line[16], NULL, 10, &error);
+ if (error != 0) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
} else {
struct http_header *hdr = talloc_zero(web, struct http_header);
char *colon = strchr(line, ':');
--
2.20.1
From 6736d39c9883506432e370a75eafd5b482dad86c 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/20] third_party: Use wrapper for string to integer
conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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..b975a1cab23 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 = 0;
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 = 0;
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 = 0;
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..b5430c8aa15 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 = 0;
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..199cd1502f8 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 = 0;
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 449a8c191bd37a73ddac8a4e3c3670bcf27f7a27 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/20] ctdb-utils: Use wrapper for string to integer
conversion
In order to detect an value overflow error during
the string to integer conversion with strtoul/strtoull,
the errno variable must be set to zero before the execution and
checked after the conversion is performed. This is achieved by
using the wrapper function strtoul_err and strtoull_err.
Signed-off-by: Swen Schillig <swen 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..a43855008c0 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 = 0;
/* 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 a9c766d23e6d3f36b9a3e23d56a9e483e949d381 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/20] 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 | 56 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 50 insertions(+), 6 deletions(-)
diff --git a/lib/util/util.c b/lib/util/util.c
index 855642c06bc..19824b550c0 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -56,17 +56,39 @@
* @param err error occured during conversion
* @result result of the conversion as provided by strtoul
*
- * Currently the only errors detected are wrong base and a value overflow.
+ * The following errors are detected
+ * - wrong base
+ * - value overflow
+ * - string with a leading "-" indicating a negative number
*/
unsigned long int
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;
@@ -81,17 +103,39 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
* @param err error occured during conversion
* @result result of the conversion as provided by strtoull
*
- * Currently the only errors detected are wrong base and a value overflow.
+ * The following errors are detected
+ * - wrong base
+ * - value overflow
+ * - string with a leading "-" indicating a negative number
*/
unsigned long long int
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;
- val = strtoull(nptr, endptr, base);
- *err = errno;
+ *err = 0;
+
+ val = strtoull(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;
--
2.20.1
From 35c3bae91b6f3ec2fd21cb867b571aeb98e76850 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Tue, 26 Feb 2019 10:28:35 +0100
Subject: [PATCH 20/20] lib: modify string conversion wrapper to handle invalid
strings
The standard string conversion routines convert a "non-number string"
into 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>
---
lib/util/util.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/lib/util/util.c b/lib/util/util.c
index 19824b550c0..f38b553c03c 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -60,6 +60,7 @@
* - wrong base
* - value overflow
* - string with a leading "-" indicating a negative number
+ * - no conversion due to empty string or not representing a number
*/
unsigned long int
strtoul_err(const char *nptr, char **endptr, int base, int *err)
@@ -78,6 +79,13 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
*endptr = tmp_endptr;
}
+ /* got an invalid number-string resulting in no conversion */
+ if (nptr == tmp_endptr) {
+ *err = EINVAL;
+ errno = saved_errno;
+ return val;
+ }
+
if (errno != 0) {
*err = errno;
errno = saved_errno;
@@ -107,6 +115,7 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
* - wrong base
* - value overflow
* - string with a leading "-" indicating a negative number
+ * - no conversion due to empty string or not representing a number
*/
unsigned long long int
strtoull_err(const char *nptr, char **endptr, int base, int *err)
@@ -125,6 +134,13 @@ strtoull_err(const char *nptr, char **endptr, int base, int *err)
*endptr = tmp_endptr;
}
+ /* got an invalid number-string resulting in no conversion */
+ if (nptr == tmp_endptr) {
+ *err = EINVAL;
+ errno = saved_errno;
+ return val;
+ }
+
if (errno != 0) {
*err = errno;
errno = saved_errno;
--
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/20190228/2598ccbf/signature.sig>
More information about the samba-technical
mailing list