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.
bool multiple;
int idx, ret;
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;
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;
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);
if (ret < 0) {
xasprintf(errctx, "option value array for --%s",
opt->name);
}
switch (opt->arg_type) {
case LLS_STRING:
}
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)
if (opt->values) {
ret = check_enum_arg(la->arg, opt, errctx);
if (ret < 0)