Global HTTP Validation Rules -> some possible improvements

[GoogleCodeExporter]

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()\\-=\\*\\.\\?;,+\\/:&_ ](1,50)$ 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]([-.\\w]*[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 reported on code.google.com by wettstei...@gmail.com on 20 Apr 2010 at 8:06

[GoogleCodeExporter]

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!

Original comment by chrisisbeef on 27 Apr 2010 at 9:17

• Changed state: Accepted
• Added labels: Component-Logic, Usability

[GoogleCodeExporter]

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

Original comment by wettstei...@gmail.com on 16 May 2010 at 8:12

Attachments:

• patch.txt

[GoogleCodeExporter]

Excellent feedback, I'd like this resolved before 2.0GA.

Original comment by manico.james@gmail.com on 2 Nov 2010 at 7:40

• Added labels: Milestone-Release2.0

[GoogleCodeExporter]

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. 

Original comment by chrisisbeef on 6 Nov 2010 at 7:58

[GoogleCodeExporter]

Chris,

These new RegEx's are very well thought out. Go forth and commit - please 
commit the test cases as well. :)

Thanks all...

Original comment by manico.james@gmail.com on 6 Nov 2010 at 8:04

[GoogleCodeExporter]

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)));

Original comment by chrisisbeef on 6 Nov 2010 at 8:19

[GoogleCodeExporter]

Cna you commit what you have so far, and I'll take a closer look now?

Original comment by manico.james@gmail.com on 6 Nov 2010 at 8:21

[GoogleCodeExporter]

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.

Original comment by manico.james@gmail.com on 6 Nov 2010 at 8:24

[GoogleCodeExporter]

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.

Original comment by chrisisbeef on 6 Nov 2010 at 8:31

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I'm reviewing this now. Thanks Chris.

Original comment by manico.james@gmail.com on 6 Nov 2010 at 8:35

[GoogleCodeExporter]

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.

Original comment by manico.james@gmail.com on 6 Nov 2010 at 8:42

[GoogleCodeExporter]

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.

Original comment by manico.james@gmail.com on 6 Nov 2010 at 9:02

[GoogleCodeExporter]

The allowNull issue is being tracked here : 
http://code.google.com/p/owasp-esapi-java/issues/detail?id=178

Original comment by manico.james@gmail.com on 6 Nov 2010 at 9:06

[GoogleCodeExporter]

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.

Original comment by kevin.w.wall@gmail.com on 6 Nov 2010 at 9:12

[GoogleCodeExporter]

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. 

Original comment by augu...@gmail.com on 9 Nov 2010 at 11:36

[GoogleCodeExporter]

August, that is an excellent idea. I agree 100%.

- Jim

Original comment by manico.james@gmail.com on 10 Nov 2010 at 8:01

[GoogleCodeExporter]

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. 

Original comment by augu...@gmail.com on 10 Nov 2010 at 10:01

[GoogleCodeExporter]

Original comment by manico.james@gmail.com on 19 Nov 2010 at 2:39

• Added labels: Validation
• Removed labels: Usability

[GoogleCodeExporter]

Based on all the comments, it sounds like this is fixed. If so, what wasn't it 
marked as fixed?

Original comment by kevin.w.wall@gmail.com on 12 Feb 2011 at 6:34

[GoogleCodeExporter]

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.\\-_]*)*/?$

Original comment by conor.j....@gmail.com on 13 Sep 2012 at 8:17

[GoogleCodeExporter]

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.

Original comment by ravikuma...@gmail.com on 21 Mar 2014 at 6:27

[GoogleCodeExporter]

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.

Original comment by ravikuma...@gmail.com on 21 Mar 2014 at 10:12

[GoogleCodeExporter]

Was this change ever published to the central repo?

Original comment by xeno6...@gmail.com on 24 Jul 2014 at 11:06

[GoogleCodeExporter]

Let's do a final validation on these patterns with SafeRegex and write some 
edge case tests and get this closed out.

Original comment by chrisisbeef on 18 Sep 2014 at 8:39

• Added labels: FirstBug

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

It fails for the following input-{"internal:getS"} for HTTPParameterValue. Any 
thoughts?

Expected output should be matches but it is failing to match. Below is the 
stack trace - 
WARN (Log4JLogger.java:449) - [SECURITY FAILURE Anonymous:null@unknown -> 
/ExampleApplication/IntrusionDetector] Invalid input: context=test, 
type(HTTPParameterValue)=^[\p{L}\p{N}.\-/+=_ !$*?@]{0,1000}$, 
input={"internal:getS"}
org.owasp.esapi.errors.ValidationException: test: Invalid input. Please conform 
to regex ^[\p{L}\p{N}.\-/+=_ !$*?@]{0,1000}$ with a maximum length of 20971520
    at org.owasp.esapi.reference.validation.StringValidationRule.checkWhitelist(StringValidationRule.java:144)
    at org.owasp.esapi.reference.validation.StringValidationRule.checkWhitelist(StringValidationRule.java:160)
    at org.owasp.esapi.reference.validation.StringValidationRule.getValid(StringValidationRule.java:284)
    at org.owasp.esapi.reference.DefaultValidator.getValidInput(DefaultValidator.java:214)
    at org.owasp.esapi.reference.DefaultValidator.isValidInput(DefaultValidator.java:152)
    at org.owasp.esapi.reference.DefaultValidator.isValidInput(DefaultValidator.java:143)
    at com.aig.appsecurity.SecurityServletFilter.doFilter(SecurityServletFilter.java:210)
    at com.ibm.ws.webcontainer.filter.FilterInstanceWrapper.doFilter(FilterInstanceWrapper.java:188)
    at com.ibm.ws.webcontainer.filter.WebAppFilterChain.doFilter(WebAppFilterChain.java:116)
    at com.ibm.ws.webcontainer.filter.WebAppFilterChain._doFilter(WebAppFilterChain.java:77)
    at com.ibm.ws.webcontainer.filter.WebAppFilterManager.doFilter(WebAppFilterManager.java:908)
    at com.ibm.ws.webcontainer.servlet.ServletWrapper.handleRequest(ServletWrapper.java:934)
    at com.ibm.ws.webcontainer.servlet.ServletWrapper.handleRequest(ServletWrapper.java:502)
    at com.ibm.ws.webcontainer.servlet.ServletWrapperImpl.handleRequest(ServletWrapperImpl.java:181)
    at com.ibm.ws.webcontainer.servlet.CacheServletWrapper.handleRequest(CacheServletWrapper.java:91)
    at com.ibm.ws.webcontainer.WebContainer.handleRequest(WebContainer.java:864)
    at com.ibm.ws.webcontainer.WSWebContainer.handleRequest(WSWebContainer.java:1592)
    at com.ibm.ws.webcontainer.channel.WCChannelLink.ready(WCChannelLink.java:186)
    at com.ibm.ws.http.channel.inbound.impl.HttpInboundLink.handleDiscrimination(HttpInboundLink.java:452)
    at com.ibm.ws.http.channel.inbound.impl.HttpInboundLink.handleNewRequest(HttpInboundLink.java:511)
    at com.ibm.ws.http.channel.inbound.impl.HttpInboundLink.processRequest(HttpInboundLink.java:305)
    at com.ibm.ws.http.channel.inbound.impl.HttpICLReadCallback.complete(HttpICLReadCallback.java:83)
    at com.ibm.ws.tcp.channel.impl.AioReadCompletionListener.futureCompleted(AioReadCompletionListener.java:165)
    at com.ibm.io.async.AbstractAsyncFuture.invokeCallback(AbstractAsyncFuture.java:217)
    at com.ibm.io.async.AsyncChannelFuture.fireCompletionActions(AsyncChannelFuture.java:161)
    at com.ibm.io.async.AsyncFuture.completed(AsyncFuture.java:138)
    at com.ibm.io.async.ResultHandler.complete(ResultHandler.java:204)
    at com.ibm.io.async.ResultHandler.runEventProcessingLoop(ResultHandler.java:775)
    at com.ibm.io.async.ResultHandler$2.run(ResultHandler.java:905)
    at com.ibm.ws.util.ThreadPool$Worker.run(ThreadPool.java:1646)
[3/20/15 15:43:39:455 EDT] 00000028 SystemOut     O InvalidInput - 
{"{internal:getS}"}

Original comment by pratikkhanna090909 on 20 Mar 2015 at 8:31