Skip to content

Conversation

@StepanGulyaev
Copy link

Hello! I was analyzing your module with Svace SAST tool and found vulnerability in set_to_pam_env function. The problem is ngx_palloc function return value not being checked though it should. Here is source code:

static void set_to_pam_env(pam_handle_t *pamh, ngx_http_request_t *r,
char *key, char *value)
{
if (key != NULL && value != NULL) {
size_t size = strlen(key) + strlen(value) + 1 * sizeof(char);
char *key_value_pair = ngx_palloc(r->pool, size);
sprintf(key_value_pair, "%s=%s", key, value);
pam_putenv(pamh, key_value_pair);
}
}

ngx_palloc is guaranteed not to return NULL only if free space exists for the variable in one of the existing pools in the linked list (refer to how ngx_palloc_small works). We cannot guarantee with absolute certainty that such space will always be available. If space is unavailable, memory allocation will be triggered: first ngx_palloc_block will be called, followed by ngx_memalign, which internally allocates memory via posix_memalign or memalign. These functions may return NULL. Therefore, ngx_palloc and functions using it should always be checked.

However because set_to_pam_env is static void function we can't just put NULL check because we should return value. Because of that my patch changes return type of two functions: set_to_pam_env and add_request_info_to_pam_env and also adds check into ngx_http_auth_pam_authenticate. I've tested it and everything works fine. If add_request_info_to_pam_env returns NGX_ERROR server will sent INTERNAL SERVER ERROR:

internal error

Found by Linux Verification Center (linuxtesting.org) with SVACE.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant