Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetEnvironmentVariable(), mismatched delete / delete[] on windows #1970

Closed
marcalff opened this issue Feb 9, 2023 · 0 comments · Fixed by #1971
Closed

GetEnvironmentVariable(), mismatched delete / delete[] on windows #1970

marcalff opened this issue Feb 9, 2023 · 0 comments · Fixed by #1971
Assignees
Labels
bug Something isn't working

Comments

@marcalff
Copy link
Member

marcalff commented Feb 9, 2023

Found by @owent.

In the main branch, in the following code:

// Returns the env variable set.
inline const std::string GetEnvironmentVariable(const char *env_var_name)
{
#if !defined(NO_GETENV)
  const char *endpoint_from_env = nullptr;
#  if defined(_MSC_VER)
  // avoid calling std::getenv which is deprecated in MSVC.
  size_t required_size = 0;
  getenv_s(&required_size, nullptr, 0, env_var_name);
  std::unique_ptr<char> endpoint_buffer;
  if (required_size > 0)
  {
    endpoint_buffer = std::unique_ptr<char>{new char[required_size]};
    getenv_s(&required_size, endpoint_buffer.get(), required_size, env_var_name);
    endpoint_from_env = endpoint_buffer.get();
  }
#  else
  endpoint_from_env = std::getenv(env_var_name);
#  endif  // defined(_MSC_VER)
  return endpoint_from_env == nullptr ? std::string{} : std::string{endpoint_from_env};
#else
  return std::string{};
#endif  // !defined(NO_GETENV)
}

This part:

  std::unique_ptr<char> endpoint_buffer;
  endpoint_buffer = std::unique_ptr<char>{new char[required_size]};

will cause an invalid call to delete when releasing the unique pointer.

The data is an array, so the code should be:

  std::unique_ptr<char[]> endpoint_buffer;
  endpoint_buffer = std::unique_ptr<char[]>{new char[required_size]};

Verified on linux, by putting the same code outside of ifdef _MSC_VER, and tested with valgrind.

==12501== Mismatched free() / delete / delete []
==12501==    at 0x4C3768B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12501==    by 0x833AFA: std::default_delete<char>::operator()(char*) const (unique_ptr.h:95)
==12501==    by 0x83382C: std::unique_ptr<char, std::default_delete<char> >::~unique_ptr() (unique_ptr.h:396)
@marcalff marcalff added the bug Something isn't working label Feb 9, 2023
@marcalff marcalff self-assigned this Feb 9, 2023
marcalff added a commit to marcalff/opentelemetry-cpp that referenced this issue Feb 9, 2023
lalitb added a commit that referenced this issue Feb 10, 2023
Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant