fix leak on one of the error branches of _dd_command_exec

Author: cataphract

appsec/src/extension/commands_helpers.c

@@ -46,6 +46,7 @@ static dd_result ATTR_WARN_UNUSED _imsg_recv(
 
 static inline ATTR_WARN_UNUSED mpack_error_t _imsg_destroy(
     dd_imsg *nonnull imsg);
+static void _imsg_cleanup(dd_imsg *nullable *imsg);
 
 static dd_result _dd_command_exec(dd_conn *nonnull conn,
     const dd_command_spec *nonnull spec, void *unspecnull ctx)
@@ -98,20 +99,21 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
             return res;
         }
 
+        // automatic cleanup of imsg on error branches
+        // set to NULL before calling _imsg_destroy
+        __attribute__((cleanup(_imsg_cleanup))) dd_imsg *nullable destroy_imsg =
+            &imsg;
+
         mpack_node_t first_response = mpack_node_array_at(imsg.root, 0);
         mpack_error_t err = mpack_node_error(first_response);
         if (err != mpack_ok) {
             mlog(dd_log_error, "Array of responses could not be retrieved - %s",
                 mpack_error_to_string(err));
-            // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
-            err = _imsg_destroy(&imsg);
             return dd_error;
         }
         if (mpack_node_type(first_response) != mpack_type_array) {
             mlog(dd_log_error, "Invalid response. Expected array but got %s",
                 mpack_type_to_string(mpack_node_type(first_response)));
-            // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
-            err = _imsg_destroy(&imsg);
             return dd_error;
         }
         mpack_node_t first_message = mpack_node_array_at(first_response, 1);
@@ -127,16 +129,12 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
         if (err != mpack_ok) {
             mlog(dd_log_error, "Response type could not be retrieved - %s",
                 mpack_error_to_string(err));
-            // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
-            err = _imsg_destroy(&imsg);
             return dd_error;
         }
         if (mpack_node_type(type) != mpack_type_str) {
             mlog(dd_log_error,
                 "Unexpected type field. Expected string but got %s",
                 mpack_type_to_string(mpack_node_type(type)));
-            // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
-            err = _imsg_destroy(&imsg);
             return dd_error;
         }
         if (dd_mpack_node_lstr_eq(type, "config_features")) {
@@ -147,8 +145,6 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
             mlog(dd_log_debug,
                 "Received message for command %.*s unexpected: %.*s\n", NAME_L,
                 (int)mpack_node_strlen(type), mpack_node_str(type));
-            // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
-            err = _imsg_destroy(&imsg);
             return dd_error;
         }
 
@@ -157,6 +153,7 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
         err = imsg.root.tree->error;
         _dump_in_msg(err == mpack_ok ? dd_log_trace : dd_log_debug, imsg._data,
             imsg._size);
+        destroy_imsg = NULL;
         err = _imsg_destroy(&imsg);
         if (err != mpack_ok) {
             mlog(dd_log_warning,
@@ -281,6 +278,14 @@ static inline ATTR_WARN_UNUSED mpack_error_t _imsg_destroy(
     return mpack_tree_destroy(&imsg->_tree);
 }
 
+static void _imsg_cleanup(dd_imsg *nullable *imsg)
+{
+    dd_imsg **imsg_c = (dd_imsg * nullable * nonnull) imsg;
+    if (*imsg_c) {
+        UNUSED(_imsg_destroy(*imsg_c));
+    }
+}
+
 /* Baked response */
 
 static void _add_appsec_span_data_frag(mpack_node_t node);