lopsub.c: Fix a NULL pointer dereference and a double free.

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.

diff --git a/lopsub.c b/lopsub.c
index 2ba26213c9c803f398208812b4069ea24321a3d4..55ea6ac56253e0705e551780271acb9be859ea25 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;