Fix for Potential memory leak in class ServiceArgumentParser's Constr… by pradishmp · Pull Request #8368 · osquery/osquery

Fixes #8351

What changed ?
This is a corner case scenario where there is a potential for a memory leak. For this leak to occur, two things must happen:

  1. The arguments passed to the service should be wide char strings.
  2. The function call toMBString, which is used to convert the wide char strings to multibyte strings, should fail, returning a nullptr.

When a nullptr is returned, as part of cleaning up the heap memory allocated, the cleanArgs() function is called. However, this function will only clean up the memory if the Boolean variable owns_argv_ptrs is set to true. In this case, as the variable is still false, the function will only remove all elements from the vector.

Later, when the Boolean variable owns_argv_ptrs is set to true and the destructor gets a chance to clean up, it cannot do anything as the elements are already removed and the vector size is zero.

Therefore, the fix is to set the owns_argv_ptrs_ = true; immediately after the CommandLineToArgvW returns successfully, thus incorporating the code review comment give by michael-myers this will speed up the memory clean up process, rather than wait for the destructor to do the job.