mod-security-users

[mod-security-users] REQUEST_URI decodes URI before running checks

[Eli <eli...@ex...>]

I didn't notice this in the documentation at first, but just ran in to =
the
outcome of not knowing about it...

I have this in my config:

        SecFilterEngine On
        SecFilterScanPOST Off
        SecFilterCheckURLEncoding Off
        SecFilterCheckUnicodeEncoding Off
        SecFilterNormalizeCookies Off
        SecFilterCheckCookieFormat Off
        SecFilterDefaultAction "log,deny,status:403"
        SecFilterSelective "REQUEST_URI" "!^[\x20-\x7E]+$"
"log,deny,status:403"

The REQUEST_URI check was my attempt to limit the characters in the URI =
(GET
requests) only, and at first it seemed just fine (restricts you to using
ASCII 32 to ASCII 126), however I just found out that mod_security =
decodes
the URI before checking it:

[Fri Feb 18 11:19:26 2005] [error] [client 216.209.84.151] mod_security:
Access denied with code 403. Pattern match "!^[\x20-\x7E]+$" at =
REQUEST_URI
[hostname "xxx"] [uri "/search.php?q=3D%A9"]

I tested and it is being caused by the "%A9" in the URI.

Now, I couldn't find out if there's a way to disable this URI decoding =
so it
instead will check exactly what the client types in as is, nor could I
figure out if there's another CGI paramater that has the same info as
REQUEST_URI but would not have it decoded before checking.

Any pointers to what I'd need?  I'm sure it's in the manual somewhere =
but I
haven't found it yet...

Oh, I'm using the dev version, so I can use all the new features in the
reference manual.

Eli.

Re: [mod-security-users] REQUEST_URI decodes URI before running checks

[Ivan R. <iv...@we...>]

Eli wrote:
> Now, I couldn't find out if there's a way to disable this URI decoding so it

   Correct, because there isn't one.

   (ducks)

   It's a curse and a blessing. In the current version it is not possible
   to disable anti-evasion techniques (URL decoding is one of them). It
   will be possible to fine-tune this behaviour in one of the future
   versions (not 1.9, most likely the one after that).

-- 
Ivan Ristic (http://www.modsecurity.org)

RE: [mod-security-users] REQUEST_URI decodes URI before running checks

[Eli <eli...@ex...>]

Ivan wrote:
> Eli wrote:
> Now, I couldn't find out if there's a way to disable this URI decoding so
it

>    Correct, because there isn't one.

Gah!

Ok, the reason for not having this yet and not having it until v2.x - is it
because of time required to work on development?  I don't mind helping...

I hacked in a flag called "SecFilterDoURLDecoding" (I think - haven't test
compiled yet) and hopefully added all the required entries for the
"dcfg->check_decoding" config paramater.  My only question is:


char *normalise(request_rec *r, sec_dir_config *dcfg, char *_uri, char
**error_msg) {
    char *uri;

    if (_uri == NULL) return NULL;
    uri = ap_pstrdup(r->pool, _uri);
    if (uri == NULL) return NULL;

    if (dcfg->check_decoding) return uri;         // is this correct?

    return normalise_inplace(r, dcfg, uri, error_msg);
}

I don't know if the "return uri" is the right way to do it or not.  Will
this cause a memory leak at all, or return something the user wouldn't
expect?

If that looks OK to you, I'm gonna test this out and see if it segs or not
:)  If not, I'll send you the patch - it's for the apache 1.x module only
though since I don't have Apache 2 to test with.  You don't have to include
it obviously - I'm sure it's a horrible hack, and it took me like 2 minutes
to hack this up so I'm sure you've got something much better planned.

Eli.

Re: [mod-security-users] REQUEST_URI decodes URI before running checks

[Ivan R. <iv...@we...>]

> Ok, the reason for not having this yet and not having it until v2.x - is it
> because of time required to work on development?  I don't mind helping...

   Two reasons: 1) it will break backward compatibility and 2) a lot of
   work is needed to do it right. No 2 is not a problem by itself but
   v2 is going to be a complete rewrite: I would much rather put that
   effort into v2 in the first place.



> I hacked in a flag called "SecFilterDoURLDecoding" (I think - haven't test
> compiled yet) and hopefully added all the required entries for the
> "dcfg->check_decoding" config paramater.  My only question is:
> 
> 
> char *normalise(request_rec *r, sec_dir_config *dcfg, char *_uri, char
> **error_msg) {
>     char *uri;
> 
>     if (_uri == NULL) return NULL;
>     uri = ap_pstrdup(r->pool, _uri);
>     if (uri == NULL) return NULL;
> 
>     if (dcfg->check_decoding) return uri;         // is this correct?
> 
>     return normalise_inplace(r, dcfg, uri, error_msg);
> }
> 
> I don't know if the "return uri" is the right way to do it or not.  Will
> this cause a memory leak at all, or return something the user wouldn't
> expect?

   It's fine. You can also move the line to the beginning of the
   function.


> to hack this up so I'm sure you've got something much better planned.

   I'd like to be able to configure individual anti-evasion methods on
   the per-rule basis. Some methods are appropriate for some things and
   not for others.

-- 
Ivan Ristic (http://www.modsecurity.org)

RE: [mod-security-users] REQUEST_URI decodes URI before running checks

[Eli <eli...@ex...>]

Hm, apparently my stuff does zilch!  It compiles, no segfaults, no =
apparent
memory leaks so far (didn't really test much in this area though), but
changing the "SecFilterDoURLDecoding" (which I noticed I reversed it's
meaning!  Heheh) to On/Off doesn't seem to make a difference at all.  I =
have
a feeling I'm missing something big here!

Eli.

------

I hacked in a flag called "SecFilterDoURLDecoding" (I think - haven't =
test
compiled yet) and hopefully added all the required entries for the
"dcfg->check_decoding" config paramater.  My only question is:


char *normalise(request_rec *r, sec_dir_config *dcfg, char *_uri, char
**error_msg) {
    char *uri;

    if (_uri =3D=3D NULL) return NULL;
    uri =3D ap_pstrdup(r->pool, _uri);
    if (uri =3D=3D NULL) return NULL;

    if (dcfg->check_decoding) return uri;         // is this correct?

    return normalise_inplace(r, dcfg, uri, error_msg);
}

I don't know if the "return uri" is the right way to do it or not.  Will
this cause a memory leak at all, or return something the user wouldn't
expect?

If that looks OK to you, I'm gonna test this out and see if it segs or =
not
:)  If not, I'll send you the patch - it's for the apache 1.x module =
only
though since I don't have Apache 2 to test with.  You don't have to =
include
it obviously - I'm sure it's a horrible hack, and it took me like 2 =
minutes
to hack this up so I'm sure you've got something much better planned.

Re: [mod-security-users] REQUEST_URI decodes URI before running checks

[Ivan R. <iv...@we...>]

Eli wrote:
> Hm, apparently my stuff does zilch!  It compiles, no segfaults, no apparent
> memory leaks so far (didn't really test much in this area though), but
> changing the "SecFilterDoURLDecoding" (which I noticed I reversed it's
> meaning!  Heheh) to On/Off doesn't seem to make a difference at all.  I have
> a feeling I'm missing something big here!

   Maybe there's code calling normalise_inplace() directly? I don't
   have the time to check right now.

   Anyway, poke around and I'm sure you'll figure it out. That's how
   I do it anyway :)

-- 
Ivan Ristic (http://www.modsecurity.org)

RE: [mod-security-users] REQUEST_URI decodes URI before running checks

[Eli <eli...@ex...>]

Ivan wrote:
> Eli wrote:
>> Hm, apparently my stuff does zilch!  It compiles, no segfaults, no
apparent
>> memory leaks so far (didn't really test much in this area though), =
but
>> changing the "SecFilterDoURLDecoding" (which I noticed I reversed =
it's
>> meaning!  Heheh) to On/Off doesn't seem to make a difference at all.  =
I
have
>> a feeling I'm missing something big here!

>   Maybe there's code calling normalise_inplace() directly? I don't
>   have the time to check right now.
>
>   Anyway, poke around and I'm sure you'll figure it out. That's how
>   I do it anyway :)

Figured it out - I was dumb :)  Since I was just copying your code and
tweaking it - I copied one line where you get the CGI variables and you =
are
forcing decoding to be done.  I didn't know it was a forcing thing, I
thought it was initializing the value to a default so I had copied my =
thing
like yours...  Of course, this was always forcing it off so that's why =
it
wasn't working!

Here's the patch - tested and it works.  Reliable?  No idea yet :)  =
About to
put it in to production on 4 webservers.


*******NOTE*******  This is not to be used by clueless users!!!  Don't =
ask
me how to use it or what it does.  It's just something to fix *MY* =
problems.


--- mod_security.c.old  Fri Feb 18 12:34:53 2005
+++ mod_security.c      Fri Feb 18 14:10:54 2005
@@ -310,6 +310,7 @@
     int filters_clear;
     int range_start;
     int range_end;
+    int check_decoding;
     int check_encoding;
     int check_unicode_encoding;
     char *upload_dir;
@@ -1372,6 +1373,7 @@

     if (dcfg->range_start =3D=3D NOT_SET) dcfg->range_start =3D 0;
     if (dcfg->range_end =3D=3D NOT_SET) dcfg->range_end =3D 255;
+    if (dcfg->check_decoding =3D=3D NOT_SET) dcfg->check_decoding =3D =
0;
     if (dcfg->check_encoding =3D=3D NOT_SET) dcfg->check_encoding =3D =
0;
     if (dcfg->check_unicode_encoding =3D=3D NOT_SET)
dcfg->check_unicode_encoding =3D 0;
     if (dcfg->upload_dir =3D=3D NOT_SET_P) dcfg->upload_dir =3D NULL;
@@ -1418,6 +1420,7 @@

     dcfg->range_start =3D NOT_SET;
     dcfg->range_end =3D NOT_SET;
+    dcfg->check_decoding =3D NOT_SET;
     dcfg->check_encoding =3D NOT_SET;
     dcfg->check_unicode_encoding =3D NOT_SET;

@@ -1489,6 +1492,7 @@

     new->range_start =3D (child->range_start =3D=3D NOT_SET) ?
parent->range_start : child->range_start;
     new->range_end =3D (child->range_end =3D=3D NOT_SET) ? =
parent->range_end :
child->range_end;
+    new->check_decoding =3D (child->check_decoding =3D=3D NOT_SET) ?
parent->check_decoding : child->check_decoding;
     new->check_encoding =3D (child->check_encoding =3D=3D NOT_SET) ?
parent->check_encoding : child->check_encoding;
     new->check_unicode_encoding =3D (child->check_unicode_encoding =
=3D=3D
NOT_SET) ? parent->check_unicode_encoding : =
child->check_unicode_encoding;
     new->upload_dir =3D (child->upload_dir =3D=3D NOT_SET_P) ? =
parent->upload_dir
: child->upload_dir;
@@ -1856,6 +1860,8 @@
 char *normalise(request_rec *r, sec_dir_config *dcfg, char *_uri, char
**error_msg) {
     char *uri;

+    if (dcfg->check_decoding) return _uri;
+
     if (_uri =3D=3D NULL) return NULL;
     uri =3D ap_pstrdup(r->pool, _uri);
     if (uri =3D=3D NULL) return NULL;
@@ -3044,6 +3050,11 @@
     return NULL;
 }

+static const char *cmd_filter_no_url_decoding(cmd_parms * cmd,
sec_dir_config *dcfg, int flag) {
+    dcfg->check_decoding =3D flag;
+    return NULL;
+}
+
 static const char *cmd_filter_check_url_encoding(cmd_parms * cmd,
sec_dir_config *dcfg, int flag) {
     dcfg->check_encoding =3D flag;
     return NULL;
@@ -3489,6 +3500,15 @@
      },

      {
+         "SecFilterNoURLDecoding",
+         cmd_filter_no_url_decoding,
+         NULL,
+         CMD_SCOPE_ANY,
+         FLAG,
+         "On or Off to set whether URL decoding will be performed"
+     },
+
+     {
          "SecFilterCheckURLEncoding",
          cmd_filter_check_url_encoding,
          NULL,

---end of patch---


Eli.

RE: [mod-security-users] REQUEST_URI decodes URI before running checks

[Eli <eli...@ex...>]

I myself wrote:
> Here's the patch - tested and it works.  Reliable?  No idea yet :)
> About to put it in to production on 4 webservers.

Here's a URL to it as well:
http://www.hoktar.com/downloads/other/mod_security-1.9dev1-no_decoding.pa=
tch

Eli.