lopsub.c: Fix a NULL pointer dereference and a double free.
authorAndre Noll <maan@tuebingen.mpg.de>
Fri, 21 Apr 2017 22:02:05 +0000 (00:02 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Wed, 12 Jul 2017 12:14:37 +0000 (14:14 +0200)
The error path of lls_deserialize_parse_result() has two issues:

* if the allocation of lor->value fails, we dereference a NULL pointer
in the cleanup part after label free_options because in

free(lor->value[j].string_val);

lor->value is NULL.

* if the strdup() for a multiple option fails in the inner loop, we
deallocate all previously allocated strings, jump to the free_options
label, and attempt to free the same values again.

The root of both bugs is that we start the cleanup in the error case
using the current value of the outer loop index i. The fix is to
perform cleanup of the allocated memory for option i already in the
allocation loop and let the cleanup loop iterate downwards from i - 1.

This bug was found by the clang static analyzer.

lopsub.c

index 2ba2621..55ea6ac 100644 (file)
--- a/lopsub.c
+++ b/lopsub.c
@@ -1296,6 +1296,7 @@ int lls_deserialize_parse_result(const char *buf, const struct lls_command *cmd,
                                if (!lor->value[j].string_val) {
                                        for (; j >= 0; j--)
                                                free(lor->value[j].string_val);
+                                       free(lor->value);
                                        goto free_options;
                                }
                                p += strlen(lor->value[j].string_val) + 1;
@@ -1313,7 +1314,7 @@ int lls_deserialize_parse_result(const char *buf, const struct lls_command *cmd,
        *lprp = lpr;
        return 1;
 free_options:
-       for (; i >= 0; i--) {
+       for (i--; i >= 0; i--) {
                const struct lls_option *opt = cmd->options + i;
                struct lls_opt_result *lor = lpr->opt_result + i;
                unsigned num_vals = (opt->flags & LLS_MULTIPLE)? lor->given : 1;