add check to parsealertargs() to avoid 64-bit overflow from used limi…

…t and unit combination
iGoogleVip · Jan 17, 2022 · 5bca736 · 5bca736
1 parent 377acfe
commit 5bca736
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 4 deletions.
diff --git a/src/vnstat_func.c b/src/vnstat_func.c
@@ -540,7 +540,7 @@ void parseargs(PARAMS *p, const int argc, char **argv)
 int parsealertargs(PARAMS *p, char **argv)
 {
 	int i, u, found, currentarg = 0;
-	uint64_t alertlimit = 0;
+	uint64_t alertlimit = 0, unitmultiplier = 1;
 	int32_t unitmode = cfg.unitmode;
 	const char *alerttypes[] = {"h", "hour", "hourly", "d", "day", "daily", "m", "month", "monthly", "y", "year", "yearly"};
 	const char *alertconditions[] = {"rx", "tx", "total", "rx_estimate", "tx_estimate", "total_estimate"}; // order must match that of AlertCondition in dbshow.h
@@ -697,11 +697,18 @@ int parsealertargs(PARAMS *p, char **argv)
 		return 0;
 	}
 
-	p->alertlimit = alertlimit * getunitdivisor(u, i);
+	unitmultiplier = getunitdivisor(u, i);
+
+	if (alertlimit > (uint64_t)(MAX64 / unitmultiplier)) {
+		printf("Error: %" PRIu64 " %s exceeds maximum supported limit of %" PRIu64 " %s.\n", alertlimit, argv[currentarg], (uint64_t)(MAX64 / unitmultiplier), argv[currentarg]);
+		return 0;
+	}
+
+	p->alertlimit = alertlimit * unitmultiplier;
 
 	if (debug) {
-		printf("Alert unit %s is %d %d = %" PRIu64 "\n", argv[currentarg], u, i, getunitdivisor(u, i));
-		printf("Alert real limit is %" PRIu64 " * %" PRIu64 " = %" PRIu64 "\n", alertlimit, getunitdivisor(u, i), p->alertlimit);
+		printf("Alert unit %s is %d %d = %" PRIu64 "\n", argv[currentarg], u, i, unitmultiplier);
+		printf("Alert real limit is %" PRIu64 " * %" PRIu64 " = %" PRIu64 "\n", alertlimit, unitmultiplier, p->alertlimit);
 	}
 
 	return 1;

diff --git a/tests/cli_tests.c b/tests/cli_tests.c
@@ -1255,6 +1255,27 @@ START_TEST(parsealertargs_can_validate_limit_unit)
 }
 END_TEST
 
+START_TEST(parsealertargs_knows_the_64_bit_limit_regardless_of_used_unit)
+{
+	int ret;
+	PARAMS p;
+	char *argv[] = {"--alert", "1", "2", "y", "total", "16", "EiB", NULL};
+
+	defaultcfg();
+	initparams(&p);
+	debug = 1;
+	suppress_output();
+
+	ret = parsealertargs(&p, argv);
+	ck_assert_int_eq(ret, 0);
+	ck_assert_int_eq(p.alertoutput, 1);
+	ck_assert_int_eq(p.alertexit, 2);
+	ck_assert_int_eq(p.alerttype, 4);
+	ck_assert_int_eq(p.alertcondition, 3);
+	ck_assert_int_eq(p.alertlimit, 0);
+}
+END_TEST
+
 START_TEST(handleshowalert_requires_interface_to_be_specified)
 {
 	PARAMS p;
@@ -1535,6 +1556,7 @@ void add_cli_tests(Suite *s)
 	tcase_add_test(tc_cli, parsealertargs_can_validate_limit_as_integer);
 	tcase_add_test(tc_cli, parsealertargs_can_validate_limit_as_non_zero);
 	tcase_add_test(tc_cli, parsealertargs_can_validate_limit_unit);
+	tcase_add_test(tc_cli, parsealertargs_knows_the_64_bit_limit_regardless_of_used_unit);
 	tcase_add_exit_test(tc_cli, handleshowalert_requires_interface_to_be_specified, 1);
 	tcase_add_test(tc_cli, validateinterface_does_not_use_alias_if_interface_names_matches);
 	tcase_add_test(tc_cli, validateinterface_supports_interface_merges);