From: Andre Noll <maan@tuebingen.mpg.de>
Date: Sun, 13 Feb 2022 19:46:20 +0000 (+0100)
Subject: lls_parse_arg(): Avoid NULL pointer dereference.
X-Git-Tag: v1.0.4~1
X-Git-Url: http://git.tue.mpg.de/?a=commitdiff_plain;h=6a4c0e8fe5ba1b8060132b12d549e82dbd3411e2;p=lopsub.git

lls_parse_arg(): Avoid NULL pointer dereference.

If a string option with multiple=false is given twice at the command
line, the second and all subsequent calls to lls_parse_arg() discard
the previous value stored in lor->value[0].

However, if the option takes an optional argument and was first
specified without argument, lor->value remains NULL because the
first call to lls_parse_arg() returned early due to the shortcut
at the beginning of the function while the second call skips the
allocation because lor->given is increased also when no argument is
given, so it equals one during the second call. Thus, the attempt
to free lor->value[0] further down in the function results in a NULL
pointer dereference.

Fix this by checking lor->value rather then lor->given. To make this
work we have to move up the code which frees the old string value.

While at it, reduce memory usage by not over-sizing the array. We
now only allocate space for idx + 1 values rather than lor->given +
1. This is different in the case mentioned above.
---

diff --git a/lopsub.c b/lopsub.c
index cbf6661..b5017c3 100644
--- a/lopsub.c
+++ b/lopsub.c
@@ -632,17 +632,23 @@ static int lls_parse_arg(struct lls_arg *la, const struct lls_option *opts,
 	bool multiple;
 	int idx, ret;
 
-	if (!la->arg)
-		goto success;
-	if (opt->arg_info == LLS_NO_ARGUMENT) {
+	if (la->arg && opt->arg_info == LLS_NO_ARGUMENT) {
 		xasprintf(errctx, "arg: %s, option: %s", la->arg, opt->name);
 		return -E_LLS_ARG_GIVEN;
 	}
 	multiple = opt->flags & LLS_MULTIPLE;
+	if (opt->arg_type == LLS_STRING && !opt->values && !multiple) {
+		if (lor->value) { /* discard previous value */
+			free(lor->value[0].string_val);
+			free(lor->value);
+			lor->value = NULL;
+		}
+	}
+	if (!la->arg)
+		goto success;
 	idx = multiple? lor->given : 0;
-	if (lor->given == 0 || multiple) {
-		ret = xrealloc(&lor->value,
-			(lor->given + 1) * sizeof(*lor->value));
+	if (!lor->value || multiple) {
+		ret = xrealloc(&lor->value, (idx + 1) * sizeof(*lor->value));
 		if (ret < 0) {
 			xasprintf(errctx, "option value array for --%s",
 				opt->name);
@@ -651,8 +657,6 @@ static int lls_parse_arg(struct lls_arg *la, const struct lls_option *opts,
 	}
 	switch (opt->arg_type) {
 	case LLS_STRING:
-		if (!opt->values && lor->given > 0 && !multiple)
-			free(lor->value[idx].string_val);
 		if (opt->values) {
 			ret = check_enum_arg(la->arg, opt, errctx);
 			if (ret < 0)