nbs-system / naxsi

NAXSI is an open-source, high performance, low rules maintenance WAF for NGINX
GNU General Public License v3.0
4.8k stars 606 forks source link

naxsi_src/naxsi_runtime.c: PVS-Studio: The 'memcpy' function doesn't copy the whole string. #376

Closed SviatoslavRazmyslov closed 7 years ago

SviatoslavRazmyslov commented 7 years ago

We have found a bug using PVS-Studio tool. PVS-Studio is a static code analyzer for C, C++ and C#: https://www.viva64.com/en/pvs-studio/

We suggests having a look at the emails, sent from @pvs-studio.com.

Analyzer warning: V575 The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null. naxsi_runtime.c 972

naxsi_src/naxsi_runtime.c:

ngx_int_t  
ngx_http_output_forbidden_page(ngx_http_request_ctx_t *ctx, 
             ngx_http_request_t *r)
{
  ....
  memcpy(h->key.data, NAXSI_HEADER_ORIG_URL, strlen(NAXSI_HEADER_ORIG_URL));
  ....
}

There might be an error in this code. The last terminal null is lost during the copying of the last string. In this case it is necessary to copy the strlen() + 1 symbol or use special functions for copying the strings: strcpy() or _strcpys().

There are many such fragments in the code, here are some of them:

blotus commented 7 years ago

Hi,

Thanks for the report, but it seems to be a false positive. The h->key.data is allocated at the line 970 (same thing for the other reports, the allocation is done a few lines before), with ngx_pcalloc() which calls ngx_memzero on the allocated buffer, so we will always a null terminator at the end of the string:

h->key.data = ngx_pcalloc(r->pool, strlen(NAXSI_HEADER_ORIG_URL)+1);