From: Martin Willi <martin@revosec.ch>
Date: Mon, 7 Jul 2014 13:49:04 +0000 (+0200)
Subject: settings: Be more strict in converting settings to specific data types
X-Git-Tag: 5.2.0~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0058e26cb0a0817bc84b9e40615ce14d247201e0;p=thirdparty%2Fstrongswan.git

settings: Be more strict in converting settings to specific data types

As the behavior was inconsistent for empty strings or strings with characters
appended to a number, testing the code failed on some platforms. The new rules
are more strict, returning the default if additional characters or an empty
string was found for a setting.
---

diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c
index 6c69782ec3..4fa283a6ff 100644
--- a/src/libstrongswan/settings/settings.c
+++ b/src/libstrongswan/settings/settings.c
@@ -509,11 +509,13 @@ METHOD(settings_t, get_bool, bool,
 inline int settings_value_as_int(char *value, int def)
 {
 	int intval;
+	char *end;
+
 	if (value)
 	{
 		errno = 0;
-		intval = strtol(value, NULL, 10);
-		if (errno == 0)
+		intval = strtol(value, &end, 10);
+		if (errno == 0 && *end == 0 && end != value)
 		{
 			return intval;
 		}
@@ -539,11 +541,13 @@ METHOD(settings_t, get_int, int,
 inline double settings_value_as_double(char *value, double def)
 {
 	double dval;
+	char *end;
+
 	if (value)
 	{
 		errno = 0;
-		dval = strtod(value, NULL);
-		if (errno == 0)
+		dval = strtod(value, &end);
+		if (errno == 0 && *end == 0 && end != value)
 		{
 			return dval;
 		}
@@ -574,6 +578,10 @@ inline u_int32_t settings_value_as_time(char *value, u_int32_t def)
 	{
 		errno = 0;
 		timeval = strtoul(value, &endptr, 10);
+		if (endptr == value)
+		{
+			return def;
+		}
 		if (errno == 0)
 		{
 			switch (*endptr)
@@ -588,8 +596,10 @@ inline u_int32_t settings_value_as_time(char *value, u_int32_t def)
 					timeval *= 60;
 					break;
 				case 's':		/* time in seconds */
-				default:
+				case '\0':
 					break;
+				default:
+					return def;
 			}
 			return timeval;
 		}
diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c
index 32676be26a..3348d33140 100644
--- a/src/libstrongswan/tests/suites/test_settings.c
+++ b/src/libstrongswan/tests/suites/test_settings.c
@@ -265,7 +265,6 @@ START_SETUP(setup_int_config)
 	create_settings(chunk_from_str(
 		"main {\n"
 		"	key1 = 5\n"
-		"	# gets cut off\n"
 		"	key2 = 5.5\n"
 		"	key3 = -42\n"
 		"	empty = \"\"\n"
@@ -283,16 +282,14 @@ END_SETUP
 START_TEST(test_get_int)
 {
 	verify_int(5, 0, "main.key1");
-	verify_int(5, 0, "main.key2");
+	verify_int(0, 0, "main.key2");
 	verify_int(-42, 0, "main.key3");
 
-	verify_int(0, 11, "main.empty");
+	verify_int(11, 11, "main.empty");
 	verify_int(11, 11, "main.none");
-
-	/* FIXME: do we want this behavior? */
-	verify_int(0, 11, "main.foo1");
-	verify_int(0, 11, "main.foo2");
-	verify_int(13, 11, "main.foo3");
+	verify_int(11, 11, "main.foo1");
+	verify_int(11, 11, "main.foo2");
+	verify_int(11, 11, "main.foo3");
 
 	verify_int(13, 13, "main.key4");
 	verify_int(-13, -13, "main");
@@ -341,13 +338,11 @@ START_TEST(test_get_double)
 	verify_double(-42, 0, "main.key3");
 	verify_double(-42.5, 0, "main.key4");
 
-	verify_double(0, 11.5, "main.empty");
+	verify_double(11.5, 11.5, "main.empty");
 	verify_double(11.5, 11.5, "main.none");
-
-	/* FIXME: do we want this behavior? */
-	verify_double(0, 11.5, "main.foo1");
-	verify_double(0, 11.5, "main.foo2");
-	verify_double(13.5, 11.5, "main.foo3");
+	verify_double(11.5, 11.5, "main.foo1");
+	verify_double(11.5, 11.5, "main.foo2");
+	verify_double(11.5, 11.5, "main.foo3");
 
 	verify_double(11.5, 11.5, "main.key5");
 	verify_double(-11.5, -11.5, "main");
@@ -375,6 +370,7 @@ START_SETUP(setup_time_config)
 {
 	create_settings(chunk_from_str(
 		"main {\n"
+		"	key0 = 5\n"
 		"	key1 = 5s\n"
 		"	key2 = 5m\n"
 		"	key3 = 5h\n"
@@ -393,18 +389,17 @@ END_SETUP
 
 START_TEST(test_get_time)
 {
+	verify_time(5, 0, "main.key0");
 	verify_time(5, 0, "main.key1");
 	verify_time(300, 0, "main.key2");
 	verify_time(18000, 0, "main.key3");
 	verify_time(432000, 0, "main.key4");
 
-	verify_time(0, 11, "main.empty");
+	verify_time(11, 11, "main.empty");
 	verify_time(11, 11, "main.none");
-
-	/* FIXME: do we want this behavior? */
-	verify_time(0, 11, "main.foo1");
-	verify_time(0, 11, "main.foo2");
-	verify_time(13, 11, "main.foo3");
+	verify_time(11, 11, "main.foo1");
+	verify_time(11, 11, "main.foo2");
+	verify_time(11, 11, "main.foo3");
 
 	verify_time(11, 11, "main.key5");
 	verify_time(11, 11, "main");