Thread: [mod-security-users] REQUEST_URI decodes URI before running checks
Brought to you by:
victorhora,
zimmerletw
From: Eli <eli...@ex...> - 2005-02-18 16:39:58
|
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. |
From: Ivan R. <iv...@we...> - 2005-02-18 17:03:17
|
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) |
From: Eli <eli...@ex...> - 2005-02-18 17:50:05
|
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. |
From: Ivan R. <iv...@we...> - 2005-02-18 18:28:31
|
> 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) |
From: Eli <eli...@ex...> - 2005-02-18 18:45:42
|
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. |
From: Ivan R. <iv...@we...> - 2005-02-18 18:49:38
|
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) |
From: Eli <eli...@ex...> - 2005-02-18 19:15:26
|
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. |
From: Eli <eli...@ex...> - 2005-02-18 19:28:56
|
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. |