Regex Syntax error on Line 106 of MustMatchRule.java

[xeno6696]

`/** * Working with request parameters. If we detect * simple regex characters, we treat it as a regex. * Otherwise we treat it as a single parameter. */ target = variable.substring(REQUEST_PARAMETERS.length());

				if ( target.contains("*") || target.contains("?") ) {

					target = target.replaceAll("*", ".*");
					Pattern p = Pattern.compile(target);`

I just tried the following unit test: ` public void testRegex() { String target = "foobar"; target = target.replaceAll("", ".*"); assertTrue("foo.*bar".equals(target)); Pattern p = Pattern.compile(target); }

`

[kwwall]

Okay... and, the the JUnit test failed because assertion failed, right? I mean you call to target.replaceAll() should have resulted in ".*f.*o.*o.*b.a.r." rather than "foo.bar". Or am I missing something? But that's very different than what is in MustMatchRule. Empty strings will be found between every character, whereas replacing "" with "." ought to do the right thing I think.

[kwwall]

Okay, I see your point. It's target.replaceAll("", ".") that throws. Hmm.

[kwwall]

I tried your test with "foo**bar" instead of "foobar" for the target and changed it to target.replaceAll("\\*", ".*") and that seems to work though. Produces "foo.*bar" as expected.

[kwwall]

So the conclusion is that no one ever used that bean shell rule before??? I don't think the WAF is enabled by default, right? Don't you have to enable org.owasp.esapi.waf.ESAPIWebApplicationFirewallFilter to get it to work? As you note, there apparently are not very complete JUnit tests for it.

[xeno6696]

Sorry Kevin for not taking time to vet this out today.

I found this with Spotbugs, and was surprised it was legit.

I only dig far enough to write that test and meant to come back to it.

I was shocked that apparently the waf “feature” is so underused that this never came to light beforehand.

[kwwall]

No big deal about the test. Spotbugs found that? Huh. That must be a known issue with String.replaceAll() then.

-kevin

Blog: http://off-the-wall-security.blogspot.com/ | Twitter: @KevinWWall NSA: All your crypto bit are belong to us.

On Tue, Nov 20, 2018, 23:43 Matt Seil <[email protected] wrote:

Sorry Kevin for not taking time to vet this out today.

I found this with Spotbugs, and was surprised it was legit.

I only dig far enough to write that test and meant to come back to it.

I was shocked that apparently the waf “feature” is so underused that this never came to light beforehand.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ESAPI/esapi-java-legacy/issues/458#issuecomment-440528398, or mute the thread https://github.com/notifications/unsubscribe-auth/AB3nm6bwCM8cVPu_isHauDRV73wVNV62ks5uxNnqgaJpZM4Yr8N6 .

[kwwall]

@xeno6696 Where do you think that such a new test should go? In src/test/java/org/owasp/esapi/waf/MustMatchTest.java perhaps? At this point (hopefully getting close to a release), I don't know enough about the WAF to be adding a bunch of new JUnit tests.

[xeno6696]

We worry about this after we push the release out. I would have released months ago, but I need that signing key!