From a1a679403ccafb26fe91c52cc4065e9e6524c09d Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Sat, 22 Apr 2017 00:02:05 +0200 Subject: [PATCH] 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. --- lopsub.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lopsub.c b/lopsub.c index 2ba2621..55ea6ac 100644 --- 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; -- 2.39.5