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

Multiple *_VAR lead to multiple matching #120

Closed
onexpiece opened this issue Apr 24, 2014 · 3 comments
Closed

Multiple *_VAR lead to multiple matching #120

onexpiece opened this issue Apr 24, 2014 · 3 comments

Comments

@onexpiece
Copy link

At naxsi_skeleton.c --> ngx_http_dummy_read_main_conf(743)

...
if (rule.br->body || rule.br->body_var) {
#ifdef main_conf_debug
    ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, 
               "pushing rule %d in body rules", rule.rule_id);  
#endif
    if (alcf->body_rules == NULL) {
      alcf->body_rules = ngx_array_create(cf->pool, 2,
                      sizeof(ngx_http_rule_t));
      if (alcf->body_rules == NULL) 
    return NGX_CONF_ERROR;
    }
    rule_r = ngx_array_push(alcf->body_rules);
    if (!rule_r) return (NGX_CONF_ERROR);
    memcpy(rule_r, &rule, sizeof(ngx_http_rule_t));
  }
...
for (i = 0; i < rule.br->custom_locations->nelts; i++) {
   ...
      if (location[i].body_var) {
    if (alcf->body_rules == NULL) {
      alcf->body_rules = ngx_array_create(cf->pool, 2,
                          sizeof(ngx_http_rule_t));
      if (alcf->body_rules == NULL) 
        return NGX_CONF_ERROR;
    }
    rule_r = ngx_array_push(alcf->body_rules);
    if (!rule_r) return (NGX_CONF_ERROR);
    memcpy(rule_r, &rule, sizeof(ngx_http_rule_t));

      }

If I have a rule like:
MainRule "str:\"" "msg:double quote" "mz:$BODY_VAR:a|URL|ARGS|$HEADERS_VAR:Cookie|$HEADERS_VAR:X-Forwarded-For" "s:$SQL:8,$XSS:8" id:1001;
alcf->body_rules will add 1001 twice according to
one if (rule.br->body || rule.br->body_var) { and one for (i = 0; i < rule.br->custom_locations->nelts; i++) {
alcf->headers_rules will add 1001 twice according to
twice for (i = 0; i < rule.br->custom_locations->nelts; i++) {

One the other hand
At naxxi_runtime.c --> ngx_http_basestr_ruleset_n(1340)

for (i = 0; i < rules->nelts && ( (!ctx->block || ctx->learning) && !ctx->drop ) ; i++) {

    /* 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) {
      location = r[i].br->custom_locations->elts;
      /* for each custom location */
      for (z = 0; z < r[i].br->custom_locations->nelts; z++) {
    /* if the name are the same, check */
    if (name->len == location[z].target.len &&
        !strncasecmp((const char *)name->data, 
             (const char *) location[z].target.data, 
             location[z].target.len)) {

#ifdef basestr_ruleset_debug
      ngx_log_debug(NGX_LOG_DEBUG_HTTP, req->connection->log, 0,
            "XX-[SPECIFIC] check one rule [%d] iteration %d * %d", r[i].rule_id, i, z);
#endif
      /* match rule against var content, */
      ret = ngx_http_process_basic_rule_buffer(value, &(r[i]), &nb_match);
      if (ret == 1) {
#ifdef basestr_ruleset_debug
        ngx_log_debug(NGX_LOG_DEBUG_HTTP, req->connection->log, 0, 
              "XX-apply rulematch [%V]=[%V] [rule=%d] (match %d times)", name, value, r[i].rule_id, nb_match); 
#endif
        ngx_http_apply_rulematch_v_n(&(r[i]), ctx, req, name, value, zone, nb_match, 0);        
      }

We don't judge where the location_custom from, body_var, headers_var or args_var.So if we have a rule like

```MainRule "str:\"" "msg:double quote" "mz:$BODY_VAR:abc|URL|ARGS|$HEADERS_VAR:Cookie|$HEADERS_VAR:X-Forwarded-For" "s:$SQL:8,$XSS:8" id:1001;```

curl -x 127.0.0.1:80 "http://www.test.com/" --data "cookie=\"123" will match too.

@buixor
Copy link
Contributor

buixor commented Apr 24, 2014

Hi !

Thanks for the detailed report, seems like a bug :)
However, I will be on hollidays for ~15 days, so I won't have time to look at this soon :/

I'll keep you posted !

@Annihil
Copy link
Contributor

Annihil commented Mar 20, 2016

Confirmed
With the latest source (Naxsi 0.54 / commit: f3e42f2)

curl http://localhost -d "cookie=\"123"
tail -f /var/log/nginx/error.log

2016/03/20 01:58:01 [error] 17455#17455: *1 NAXSI_FMT: ip=127.0.0.1&server=localhost&uri=/&learning=1&vers=0.54&total_processed=1&total_blocked=1&block=1&cscore0=$SQL&score0=16&cscore1=$XSS&score1=16&zone0=BODY&id0=1001&var_name0=cookie&zone1=BODY&id1=1001&var_name1=cookie, client: 127.0.0.1, server: _, request: "POST / HTTP/1.1", host: "localhost"

There are two problems here:

  • the variable cookie, here, is a BODY var, so it should not match with $HEADERS_VAR:Cookie
  • there are two matches, but there should only be one

@buixor
Copy link
Contributor

buixor commented Mar 24, 2016

Thanks for the report, I fixed the bug (even somehow it was not a bug, but a side-effect :p).
I will wait a bit before pushing it upstream, to ensure that it doesn't break anything :)

@buixor buixor closed this as completed in 3cc10b9 Mar 24, 2016
denji added a commit to denji/homebrew-nginx that referenced this issue Apr 5, 2016
* Added support for RAW_BODY (rules to be matched against the full, raw
body. Can be useful to match rules against unparsed content : XML,
serialized java objects etc.)
* Confirmed support as a dynamic module (introduced in nginx 1.9.11)
* Better makefile for testing & dev, increased coverage
* Minor bug-fixes (nbs-system/naxsi#120, nbs-system/naxsi#241,
nbs-system/naxsi#217, nbs-system/naxsi#231)
denji added a commit to denji/homebrew-nginx that referenced this issue Apr 5, 2016
* Added support for RAW_BODY (rules to be matched against the full, raw
  body. Can be useful to match rules against unparsed content : XML,
  serialized java objects etc.)
* Confirmed support as a dynamic module (introduced in nginx 1.9.11)
* Better makefile for testing & dev, increased coverage
* Minor bug-fixes (nbs-system/naxsi#120, nbs-system/naxsi#241,
  nbs-system/naxsi#217, nbs-system/naxsi#231)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants