Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Commit

Permalink
Collision reduce (#401)
Browse files Browse the repository at this point in the history 
* fix collision on vpatch where URL context was sometime ignored

* update nginx vers

* test case

* remove debug
  • Loading branch information
@buixor @blotus
buixor authored and blotus committed Nov 6, 2017
1 parent ad88be3 commit ec4ce3e
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 16 deletions.
2 changes: 1 addition & 1 deletion naxsi_src/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ STOCK ?= 1

#Allows to force for specific UT only
#TEST := ""
NGINX_VERS := "1.10.0"
NGINX_VERS := "1.12.2"


NGINX_OPTIONS="--with-select_module"
Expand Down
38 changes: 23 additions & 15 deletions naxsi_src/naxsi_runtime.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1184,8 +1184,6 @@ ngx_http_spliturl_ruleset(ngx_pool_t *pool,
int len, full_len;
int nullbytes=0;

NX_DEBUG(_debug_spliturl_ruleset, NGX_LOG_DEBUG_HTTP, req->connection->log, 0,
"XX-check url-like [%s]", str);


if (naxsi_escape_nullbytes(nx_str) > 0) {
Expand All @@ -1196,6 +1194,9 @@ ngx_http_spliturl_ruleset(ngx_pool_t *pool,
}
str = (char *)nx_str->data;

NX_DEBUG(_debug_spliturl_ruleset, NGX_LOG_DEBUG_HTTP, req->connection->log, 0,
"XX-check url-like [%s]", str);

orig = str;
full_len = strlen(orig);
while (str < (orig+full_len) && *str) {
Expand Down Expand Up @@ -1403,7 +1404,7 @@ ngx_http_basestr_ruleset_n(ngx_pool_t *pool,
"XX-RULE %d : START", r[i].rule_id);

/* does the rule have a custom location ? custom location means checking only on a specific argument */
if (name && name->len > 0 && r[i].br->custom_location) {
if (name && r[i].br->custom_location) {
location = r[i].br->custom_locations->elts;

/*
Expand All @@ -1415,18 +1416,23 @@ ngx_http_basestr_ruleset_n(ngx_pool_t *pool,
*/
for (z = 0; z < r[i].br->custom_locations->nelts; z++) {

if (location[z].specific_url) {

if (location[z].specific_url) {
/* if matchzone is a regex, ensure it matches (ie. BODY_VAR_X / ARGS_VAR_X / ..) */
if (r[i].br->rx_mz && ngx_http_dummy_pcre_wrapper(location[z].target_rx, req->uri.data, req->uri.len) == -1)
uri_constraint_ok = 0;
if (r[i].br->rx_mz) {

if (ngx_http_dummy_pcre_wrapper(location[z].target_rx, req->uri.data, req->uri.len) == -1) {
uri_constraint_ok = 0;
}
}

/* if it was a static string, ensure it matches (ie. BODY_VAR / ARGS_VAR / ..) */
if ( (!r[i].br->rx_mz) && strncasecmp((const char *) req->uri.data,
(const char *) location[z].target.data,
req->uri.len) )
uri_constraint_ok = 0;

if (!r[i].br->rx_mz) {
if (req->uri.len != location[z].target.len || strncasecmp((const char *) req->uri.data,
(const char *) location[z].target.data,
req->uri.len) != 0) {
uri_constraint_ok = 0;
}
}
break;
}
}
Expand All @@ -1435,8 +1441,11 @@ ngx_http_basestr_ruleset_n(ngx_pool_t *pool,
** if one of the custom location rule specifies an $URL/$URL_X
** and it was mismatched, skip the rule.
*/
if (uri_constraint_ok == 0)
if (uri_constraint_ok == 0) {
NX_DEBUG(_debug_basestr_ruleset , NGX_LOG_DEBUG_HTTP, req->connection->log, 0,
"XX URI CONSTRAINT MISMATCH, SKIP");
continue;
}

/* for each custom location */
for (z = 0; z < r[i].br->custom_locations->nelts; z++) {
Expand All @@ -1447,7 +1456,7 @@ ngx_http_basestr_ruleset_n(ngx_pool_t *pool,
!(zone == HEADERS && location[z].headers_var != 0) &&
!(zone == ARGS && location[z].args_var != 0))
continue;

/* if matchzone is a regex, ensure it matches (ie. BODY_VAR_X / ARGS_VAR_X / ..) */
if (r[i].br->rx_mz && ngx_http_dummy_pcre_wrapper(location[z].target_rx, name->data, name->len) == -1)
continue;
Expand All @@ -1459,7 +1468,6 @@ ngx_http_basestr_ruleset_n(ngx_pool_t *pool,
location[z].target.len)) )
continue;


NX_DEBUG(_debug_basestr_ruleset, NGX_LOG_DEBUG_HTTP, req->connection->log, 0,
"XX-[SPECIFIC] check one rule [%d] iteration %d * %d", r[i].rule_id, i, z);

Expand Down
67 changes: 67 additions & 0 deletions t/29regression.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,70 @@ use URI::Escape;
"POST /wp-json/wp/v2/posts/111
id=1a&foo2=bar2"
--- error_code: 412
=== WL TEST 3.0: false-positive on virtual-patch with empty var name
--- main_config
load_module /tmp/naxsi_ut/modules/ngx_http_naxsi_module.so;
--- http_config
include /tmp/naxsi_ut/naxsi_core.rules;
MainRule "rx:FOOBAR" "mz:$URL:/wp-includes/js/plupload/plupload.flash.swf|ARGS" "msg:Wordpress PlUpload XSS" "s:$UWA:8,$XSS_UWA:1" id:42000485;
--- config
location / {
SecRulesEnabled;
CheckRule "$LOG_TEST >= 1" LOG;
CheckRule "$UWA >= 8" BLOCK;
DeniedUrl "/RequestDenied";
CheckRule "$SQL >= 4" BLOCK;
root $TEST_NGINX_SERVROOT/html/;
index index.html index.htm;
}
location /RequestDenied {
return 412;
}
--- request
GET /?a=bui&FOOBAR
--- error_code: 200
=== WL TEST 3.0: false-positive on virtual-patch with empty var name
--- main_config
load_module /tmp/naxsi_ut/modules/ngx_http_naxsi_module.so;
--- http_config
include /tmp/naxsi_ut/naxsi_core.rules;
MainRule "rx:FOOBAR" "mz:$URL:/wp-includes/js/plupload/plupload.flash.swf|ARGS" "msg:Wordpress PlUpload XSS" "s:$UWA:8,$XSS_UWA:1" id:42000485;
--- config
location / {
SecRulesEnabled;
CheckRule "$LOG_TEST >= 1" LOG;
CheckRule "$UWA >= 8" BLOCK;
DeniedUrl "/RequestDenied";
CheckRule "$SQL >= 4" BLOCK;
root $TEST_NGINX_SERVROOT/html/;
index index.html index.htm;
}
location /RequestDenied {
return 412;
}
--- request
GET /wp-includes/js/plupload/plupload.flash.swf?a=bui&FOOBAR
--- error_code: 412
=== WL TEST 3.01: false-positive on virtual-patch with empty var name
--- main_config
load_module /tmp/naxsi_ut/modules/ngx_http_naxsi_module.so;
--- http_config
include /tmp/naxsi_ut/naxsi_core.rules;
MainRule "rx:FOOBAR" "mz:$URL:/wp-includes/js/plupload/plupload.flash.swf|ARGS" "msg:Wordpress PlUpload XSS" "s:$UWA:8,$XSS_UWA:1" id:42000485;
--- config
location / {
SecRulesEnabled;
CheckRule "$LOG_TEST >= 1" LOG;
CheckRule "$UWA >= 8" BLOCK;
DeniedUrl "/RequestDenied";
CheckRule "$SQL >= 4" BLOCK;
root $TEST_NGINX_SERVROOT/html/;
index index.html index.htm;
}
location /RequestDenied {
return 412;
}
--- request
GET /wp-includes/js/plupload/plupload.flash.swf/xxx/?a=bui&FOOBAR
--- error_code: 404

0 comments on commit ec4ce3e

Please sign in to comment.