Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mismatch in handling of unescaped backslashes #177

Closed
vorner opened this issue Jun 8, 2021 · 4 comments
Closed

Mismatch in handling of unescaped backslashes #177

vorner opened this issue Jun 8, 2021 · 4 comments

Comments

@vorner
Copy link

vorner commented Jun 8, 2021

Hello

It seems the rust parser and suricata behaves differently when presented with backslashes in rule option values that are not escaped. Suricata seems to preserve them, while the evebox-suricata-rule-parser strips them.

This often leads to damaging pcre options, as they often contain backslashes that are significant for the regex itself. One that's in the example in the documentation: https://suricata.readthedocs.io/en/suricata-6.0.0/rules/payload-keywords.html?highlight=pcre#pcre-perl-compatible-regular-expressions: pcre: "/^/index\.html/$/U". Apparently, the \. is meant to match literal dot and not any character.

This is how it gets parsed:

fn main() {
    const RULE: &str = r#"alert http $HOME_NET any -> $EXTERNAL_NET any (msg:""; http_uri; content:"index."; pcre: "/^/index\.html/$/U"; sid:9000000; rev:1;)"#;
    let rule = evebox_suricata_rule_parser::parse_rule(RULE).unwrap();
    dbg!(rule);
}
        RuleOption {
            key: "pcre",
            val: Some(
                "/^/index.html/$/U",
            ),
        },
jasonish added a commit that referenced this issue Jun 8, 2021
Re-add '\' when found to not be escaping a double quote.

GitHub issue:
#177
@jasonish
Copy link
Owner

jasonish commented Jun 8, 2021

The commit reference above fixes this issue, it was in the quote stripping. However, as discussed in #176, not stripping the quotes at all fixes this issue. However, a function to strip quotes would still be a useful utility method, so will fix this.

@vorner
Copy link
Author

vorner commented Jun 9, 2021

Hello

Thanks for the super-fast fix. But as I'm looking at the code, it seems ; is also special and should eat the backslash in front of it:

https://suricata.readthedocs.io/en/suricata-6.0.0/rules/intro.html#rule-options

Hopefully, that's all of them.

@jasonish
Copy link
Owner

jasonish commented Jun 9, 2021

Thanks for the super-fast fix. But as I'm looking at the code, it seems ; is also special and should eat the backslash in front of it:

This should already happen in parse_option and has a test here: https://github.com/jasonish/evebox/blob/master/suricata-rule-parser/src/lib.rs#L408

@jasonish
Copy link
Owner

Closing. Fixed with 0b63979.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants