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

Possible null dereference in opentracing_variable.cpp #669

Open
suhovv opened this issue Aug 26, 2024 · 2 comments
Open

Possible null dereference in opentracing_variable.cpp #669

suhovv opened this issue Aug 26, 2024 · 2 comments

Comments

@suhovv
Copy link

suhovv commented Aug 26, 2024

File: opentracing_variable.cpp
line: 108, 115

To avoid possible nullptr dereference, you should add a check for the result of the ngx_http_add_variable function call. If the function returns nullptr, you should handle this situation correctly, for example, write an error message to the log and return the corresponding error code from the add_variables function.

For the line opentracing_binary_context_var->get_handler = expand_opentracing_binary_context_variable; the same principles of error checking and handling apply as in the case of the opentracing_context_var variable

Example of correction:

ngx_int_t add_variables(ngx_conf_t* cf) noexcept {
  // add opentracing_context
  auto opentracing_context = to_ngx_str(opentracing_context_variable_name);
  auto opentracing_context_var = ngx_http_add_variable(
      cf, &opentracing_context,
      NGX_HTTP_VAR_NOCACHEABLE | NGX_HTTP_VAR_NOHASH | NGX_HTTP_VAR_PREFIX);
  
  // check nullptr
  if (opentracing_context_var == nullptr) {
    ngx_log_error(NGX_LOG_ERR, cf->log, 0, "Failed to add variable: %V", &opentracing_context);
    return NGX_ERROR;
  }

  opentracing_context_var->get_handler = expand_opentracing_context_variable;
  opentracing_context_var->data = 0;

  // add opentracing_binary_context
  auto opentracing_binary_context =
      to_ngx_str(opentracing_binary_context_variable_name);
  auto opentracing_binary_context_var = ngx_http_add_variable(
      cf, &opentracing_binary_context, NGX_HTTP_VAR_NOCACHEABLE);

  // check nullptr
  if (opentracing_binary_context_var == nullptr) {
    ngx_log_error(NGX_LOG_ERR, cf->log, 0, "Failed to add variable: %V", &opentracing_binary_context);
    return NGX_ERROR;
  }

  opentracing_binary_context_var->get_handler =
      expand_opentracing_binary_context_variable;
  opentracing_binary_context_var->data = 0;

  return NGX_OK;
}
@opentracing-contrib opentracing-contrib deleted a comment Aug 26, 2024
@miry
Copy link
Collaborator

miry commented Aug 26, 2024

@snisorig Thank you for reporting. Do you think you can prepare a PR with tests to cover that problem?

@suhovv
Copy link
Author

suhovv commented Aug 26, 2024

@miry Sorry, but I have little experience in this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants