esapi-java-legacy
esapi-java-legacy copied to clipboard
ESAPI
Global HTTP Validation Rules -> some possible improvements
From [email protected] on April 20, 2010 04:06:42
I'm a thankful user of the SafeRequest (1.4, in 2.0 SecurityWrapperRequest) which offers a very good protection against various kinds of injection attacks.
I have some suggestions for improvements concerning the regular expressions in use.
Validator.HTTPParameterName=^[a-zA-Z0-9_]{1,32}$ I would add the "-", since some frameworks (like DisplayTag) create ParameterNames of the kind "d-32143423-s".
Validator.HTTPParameterValue=^[a-zA-Z0-9.-/+=_ ]$ European languages, like French or German, take frequently use of special characters like [äüöéèàç]. These are not covered here. Furthermore, the regular expression is inconsistent with the output of ESAPI.authenticator().generateStrongPassword(). A password generated by this method would not pass the above RegEx. The following special chars have to be added: '!', '$', '', '?', '@'
Validator.HTTPQueryString=^a-zA-Z0-9()-=*.?;,+/:&_ $ The (1,50) at the end should be {1,50}. Furthermore 50 characters seem to be a bit short, since most modern browsers support queryString of more than 2000 characters.
Validator.HTTPContextPath=^[a-zA-Z0-9.-]*$ Tomcat (it may depend on the configuration) returns "/application". So it is necessary to add “/” at the beginning of the regular expression: Validator.HTTPContextPath=^/[a-zA-Z0-9.-]*$. The same is true for Validator.HTTPServletPath
Validator.HTTPURL=^.$ Why not use Validator.URL=^(ht|f)tp(s?)://0-9a-zA-Z(:(0-9))(/?)([a-zA-Z0-9-.?,:'/\+=&%$#_]*)?$ instead?
I have seen that there only a few tests in SafeRequestTest.java. If you are looking for someone to add some further TestCases, feel free to contact me.
Original issue: http://code.google.com/p/owasp-esapi-java/issues/detail?id=116
From chrisisbeef on April 27, 2010 14:17:44
Thanks for the great suggestions!
I will take some time to run over these RegEx's this evening - in the meantime, if you have test cases, we can always use more! Feel free to attach them as a patch to this issue and I will take a look at them and get them integrated.
Thanks again!
Status: Accepted
Owner: chrisisbeef
Labels: Component-Logic Usability
From [email protected] on May 16, 2010 13:12:02
Hi again!
I've written some additional testcases (see attached file), I hope you can use them. Here are the regular expressions which I've modified for the tests to pass.
Validator.HTTPParameterName=^[a-zA-Z0-9_-]{1,32}$ Validator.HTTPParameterValue=^[\p{L}\p{N}.-/+=_ !$?@]{0,1000}$ Validator.HTTPContextPath=^/[a-zA-Z0-9.-]$ Validator.HTTPQueryString=^([a-zA-Z0-9-]{1,32}=[\p{L}\p{N}.-/+=_ !$?@]&?)$ Validator.HTTPURI=^/([a-zA-Z0-9.-]_/?)*$
Frank
Attachment: patch.txt
From [email protected] on November 02, 2010 00:40:34
Excellent feedback, I'd like this resolved before 2.0GA.
Labels: Milestone-Release2.0
From chrisisbeef on November 06, 2010 00:58:07
Can we get a couple of stars on this if the regex's look good - if all is well I will commit new regex's.
Cc: kevin.w.wall manico.james planetlevel [email protected]
From [email protected] on November 06, 2010 01:04:41
Chris,
These new RegEx's are very well thought out. Go forth and commit - please commit the test cases as well. :)
Thanks all...
From chrisisbeef on November 06, 2010 01:19:29
in test: public void testisValidInput() {
this line fails - regex is allowing the * character as a valid character in this context. Can you verify if this is correct and the test needs to be changed or if this is incorrect and something is wrong with the regex.
assertFalse(instance.isValidInput("test", "jeff*WILLIAMS", "HTTPParameterValue", 100, false));
in test: public void testGetParameter() {
this line throws a NPE:
assertFalse(safeRequest.getParameter("f" + i).equals(request.getParameter("f" + i)));
From [email protected] on November 06, 2010 01:21:44
Cna you commit what you have so far, and I'll take a closer look now?
Cc: -kevin.w.wall -planetlevel [email protected]
From [email protected] on November 06, 2010 01:24:44
Chris, the test is wrong. Check out his regex...
Validator.HTTPParameterValue=^[\p{L}\p{N}.-/+=_ !$*?@]{0,1000}$
...it's allowing *. The revised regEx came AFTER he submitted unit tests. I say, fix the test and charge on.
From chrisisbeef on November 06, 2010 01:31:30
These changes have only been applied to the ESAPI.properties under src/test/resources to ensure that all issues are resolved before integrating into distributed configuration.
Updated parameter test to allow '*'
The second aforementioned test is still throwing NullPointerException
All code and changes checked in for this.
From [email protected] on November 06, 2010 01:42:42
Chris,
Those secondary tests should all fail. ("f" + i) f==fail.
assertFalse(safeRequest.getParameter("f" + i).equals(request.getParameter("f" + i)));
We just need a tiny cleanup here. I'm working on it now.
From [email protected] on November 06, 2010 02:02:29
I fixed these unit tests, please update to see. These are not complete (as in, these tests revealed other issues). I'm creating new Google code bugs to discuss other revealed problems. But the code submitted by the original poster is good, I think we should commit the RegEx's to the main of trunk.
From [email protected] on November 06, 2010 02:06:53
The allowNull issue is being tracked here : https://code.google.com/p/owasp-esapi-java/issues/detail?id=178
From [email protected] on November 06, 2010 14:12:23
These regexs can be simplified a bit. Specifically, in a character class, marked by [...], if '-' is the first or last character in the character class, then it does not have to be quoted because it has no special meaning in those positions. Therefore, I would recommend that we change these proposed regexs to more the '-' to the end. E.g., instead of: Validator.HTTPContextPath=^[a-zA-Z0-9.-]*$ use Validator.HTTPContextPath=^[a-zA-Z0-9.-]*$
I generally try to avoid backslashes in regexs because depending on the processing that goes on before they are turned into a compiled pattern the # of backslashes change. Hence when backslashes are used within a regex to quote something you make your code much more fragile.
From [email protected] on November 09, 2010 15:36:36
These changes now cause SafeRequestTest.testGetQueryStringPercent to fail. The new regex for Validator.HTTPQueryString does not include the % character:
Validator.HTTPQueryString=^([a-zA-Z0-9_-]{1,32}=[\p{L}\p{N}.-/+=_ !$?@]&?)*$
if % is not included in the regex, and a request parameter is percent-encoded ESAPI.validator().getValidInput() will return null, causing the test to fail.
The solution is to add the % character to the regex:
Validator.HTTPQueryString=^([a-zA-Z0-9_-]{1,32}=[\p{L}\p{N}.-/+=_ !$?@%]&?)*$
A better approach to SecurityWrapperRequest.getQueryString() would probably be to get the parameter names, validate each one individually, then validate the value for each parameter name, and assemble them into a safe query string.
From [email protected] on November 10, 2010 00:01:00
August, that is an excellent idea. I agree 100%.
- Jim
From [email protected] on November 10, 2010 14:01:56
I checked in my validator change so at least unit tests will now pass.
These changes still need further testing and to be rolled into the live version of ESAPI.properties.
From [email protected] on February 11, 2011 22:34:50
Based on all the comments, it sounds like this is fixed. If so, what wasn't it marked as fixed?
From [email protected] on September 13, 2012 01:17:56
Regarding this particular regex:
Validator.HTTPURI=^/([a-zA-Z0-9.-]/?)_$
I wrote a test case for some code that uses it, to assert it would not accept query params and some other invalid uris
"/AbcTestApp/emf/report/renderInitialView.do/abc?a=5".matches(httpUriRegex)
Lo and behold it hadn't returned an answer after 90 minutes. (java 1.6) Could be a performance bug with the regex backtracking, but it seems like something in the regex engine.
I tweaked the regex, removing the optional trailing slash from the repeating part of the reg ex (moving the initial slash inside the group to do the same job). Now there is nothing optional inside the repeating group. It seems to work fine old=^/([a-zA-Z0-9.-]/?)$ new=^(/[a-zA-Z0-9.-])/?$
From [email protected] on March 20, 2014 23:27:10
Hi Team,
Iam ravikumar and iam new to use ESAPI. iam using esapi approach for few parameters. if the value is coming as #, then in ui (java) it is displaying as encoding value, i.e. %23. As per the business requirement we need to pass #, $ as a parameters. please let me know where do i change, so that encoded values should not display in the front end.
From [email protected] on March 21, 2014 03:12:59
Hi Team,
Iam ravikumar and iam new to use ESAPI. iam using esapi approach for few parameters. if the value is coming as #, then in ui (java) it is displaying as encoding value, i.e. %23. As per the business requirement we need to pass #, $ as a parameters. please let me know where do i change, so that encoded values should not display in the front end.
From [email protected] on July 24, 2014 16:06:33
Was this change ever published to the central repo?
From chrisisbeef on September 18, 2014 13:39:20
Let's do a final validation on these patterns with SafeRegex and write some edge case tests and get this closed out.
Labels: FirstBug