Skip to content

Regex Syntax error on Line 106 of MustMatchRule.java #458

Open

Description

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

`

Activity

  1. kwwall commented on Nov 21, 2018

    @kwwall
    Contributor

    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.

  2. kwwall commented on Nov 21, 2018

    @kwwall
    Contributor

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

  3. kwwall commented on Nov 21, 2018

    @kwwall
    Contributor

    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.

  4. kwwall commented on Nov 21, 2018

    @kwwall
    Contributor

    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.

  5. xeno6696 commented on Nov 21, 2018

    @xeno6696
    CollaboratorAuthor

    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.

  6. kwwall commented on Nov 21, 2018

    @kwwall
    Contributor
  7. kwwall commented on Nov 23, 2018

    @kwwall
    Contributor

    @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.

  8. xeno6696 commented on Nov 24, 2018

    @xeno6696
    CollaboratorAuthor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions