Skip to content

Conversation

@StepanGulyaev
Copy link

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

static char* ngx_strncpy_s(ngx_str_t source, ngx_pool_t *pool)
{
// allocate memory in pool
char* destination = ngx_palloc(pool, source.len + 1);
strncpy(destination, (char *) source.data, source.len);
// add null terminator
destination[source.len] = '\0';
return destination;
}

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.

I've changed function return type, now it returns NGX_OK or NGX_ERROR and char* variable is passed as one of the arguments. If allocation failed server will return NGX_HTTP_INTERNAL_SERVER_ERROR. The chance it will happen is pretty low but it still may happen.

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