[PATCH weston v2 2/2] Re-apply "config-parser: Catch negative numbers assigned to unsigned config values"

Bryce Harrington bryce at osg.samsung.com
Fri Jul 15 01:28:04 UTC 2016


[With hexadecimal color values now handled via their own routine,
re-introduce the negative unsigned numbers fix.]

strtoul() has a side effect that when given a string representing a
negative number, it treats it as a high value hexadecimal.  IOW,
strtoul("-42", &val) sets val to 0xffffffd6.  This could potentially
result in unintended surprise behaviors.

Catch this by using strtol() and then manually check for the negative
value.  This logic is modelled after Wayland's strtouint().

Note that this change unfortunately reduces the range of parseable
numbers from [0,UINT_MAX] to [0,INT_MAX].  The current users of
weston_config_section_get_uint() are anticipating numbers far smaller
than either of these limits, so the change is believed to have no impact
in practice.

Also add a test case for negative numbers that catches this error
condition.

Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
---
 shared/config-parser.c     | 12 +++++++++++-
 tests/config-parser-test.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/shared/config-parser.c b/shared/config-parser.c
index 33d5e36..d5bbb8d 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -186,6 +186,7 @@ weston_config_section_get_uint(struct weston_config_section *section,
 			       const char *key,
 			       uint32_t *value, uint32_t default_value)
 {
+	long int ret;
 	struct weston_config_entry *entry;
 	char *end;
 
@@ -197,13 +198,22 @@ weston_config_section_get_uint(struct weston_config_section *section,
 	}
 
 	errno = 0;
-	*value = strtoul(entry->value, &end, 0);
+	ret = strtol(entry->value, &end, 0);
 	if (errno != 0 || end == entry->value || *end != '\0') {
 		*value = default_value;
 		errno = EINVAL;
 		return -1;
 	}
 
+	/* check range */
+	if (ret < 0 || ret > INT_MAX) {
+		*value = default_value;
+		errno = ERANGE;
+		return -1;
+	}
+
+	*value = ret;
+
 	return 0;
 }
 
diff --git a/tests/config-parser-test.c b/tests/config-parser-test.c
index 5a5a380..7f30b22 100644
--- a/tests/config-parser-test.c
+++ b/tests/config-parser-test.c
@@ -117,6 +117,7 @@ static struct zuc_fixture config_test_t1 = {
 	"# more comments\n"
 	"number=5252\n"
 	"zero=0\n"
+	"negative=-42\n"
 	"flag=false\n"
 	"\n"
 	"[colors]\n"
@@ -578,6 +579,36 @@ ZUC_TEST_F(config_test_t1, test026, data)
 	ZUC_ASSERT_EQ(EINVAL, errno);
 }
 
+ZUC_TEST_F(config_test_t1, test025, data)
+{
+	int r;
+	int32_t n;
+	struct weston_config_section *section;
+	struct weston_config *config = data;
+
+	section = weston_config_get_section(config, "bar", NULL, NULL);
+	r = weston_config_section_get_int(section, "negative", &n, 600);
+
+	ZUC_ASSERT_EQ(0, r);
+	ZUC_ASSERT_EQ(-42, n);
+	ZUC_ASSERT_EQ(0, errno);
+}
+
+ZUC_TEST_F(config_test_t1, test026, data)
+{
+	int r;
+	uint32_t n;
+	struct weston_config_section *section;
+	struct weston_config *config = data;
+
+	section = weston_config_get_section(config, "bar", NULL, NULL);
+	r = weston_config_section_get_uint(section, "negative", &n, 600);
+
+	ZUC_ASSERT_EQ(-1, r);
+	ZUC_ASSERT_EQ(600, n);
+	ZUC_ASSERT_EQ(ERANGE, errno);
+}
+
 ZUC_TEST_F(config_test_t2, doesnt_parse, data)
 {
 	struct weston_config *config = data;
-- 
1.9.1



More information about the wayland-devel mailing list