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

Cannot get REQUEST_FILENAME with %3f (encoded ?) in url path #2705

Open
liudongmiao opened this issue Mar 19, 2022 · 2 comments
Open

Cannot get REQUEST_FILENAME with %3f (encoded ?) in url path #2705

liudongmiao opened this issue Mar 19, 2022 · 2 comments

Comments

@liudongmiao
Copy link
Contributor

Describe the bug

If the url path contains %3f, cannot get real REQUEST_FILENAME.

Logs and dumps

ModSecurity: Warning. Matched "Operator `Gt' with parameter `0' against variable `REQUEST_URI_RAW' (Value: `/path1%3fpath2?query=%3f' ) [file "..."] [line "1"] [id "1"] [rev ""] [msg ""] [data ""] [severity "0"] [ver ""] [maturity "0"] [accuracy "0"] [hostname "127.0.0.1"] [uri "/path1"] [unique_id "..."] [ref "v4,24t:length"]
ModSecurity: Warning. Matched "Operator `Gt' with parameter `0' against variable `REQUEST_FILENAME' (Value: `/path1' ) [file "..."] [line "2"] [id "2"] [rev ""] [msg ""] [data ""] [severity "0"] [ver ""] [maturity "0"] [accuracy "0"] [hostname "127.0.0.1"] [uri "/path1"] [unique_id "..."] [ref "v4,14t:length"]

To Reproduce

Test for path: /path1%3fpath2?query=%3f

Expected behavior

Return urldecoded or original filename.

Server (please complete the following information):

  • ModSecurity version (and connector): ModSecurity v3.0.6

Rule Set (please complete the following information):

SecRule REQUEST_URI_RAW "@gt 0" "id:1,phase:1,t:length,pass,log,auditlog"
SecRule REQUEST_FILENAME "@gt 0" "id:2,phase:1,t:length,pass,log,auditlog"

Additional context

Add any other context about the problem here.

@liudongmiao
Copy link
Contributor Author

Current source codes.

    size_t pos_query = m_uri_decoded.find("?");

    /* ... skip multi lines ... */

    std::string path_info;
    if (pos_query == std::string::npos) {
        path_info = std::string(m_uri_decoded, 0);
    } else {
        path_info = std::string(m_uri_decoded, 0, pos_query);
    }
    if (var_size == std::string::npos) {
        var_size = uri_s.size();
    }

@martinhsv
Copy link
Contributor

It has been a longstanding practice to do some automatic url decoding of REQUEST_FILENAME. One of the reasons that REQUEST_URI_RAW exists is so that an entirely undecoded string is available for those (expected to be very rare) occasions when that is needed.

Because of the availability of an alternative, the long history of decoding, as well as basic matching of web server behaviour, it probably wouldn't be a good idea for us to consider changing this functionality to never automatically perform url of REQUEST_FILENAME.

That said, the current v3 behaviour does result in two anomalies.

The first of these is what has been highlighted by the OP in this ticket: that if the path portion of the uri contains a literal '?' character (encoded in the request as %3b) then ModSecurity will truncate the REQUEST_FILENAME. This would qualify as a bug, but presumably one that would occur so rarely as to be almost never.

A second sub-issue is perhaps less of a true bug, but at least a notable difference from ModSecurity v2 behaviour. In that version, the REQUEST_FILENAME variable is indeed decoded if accessed in a phase:2 rule, but it is undecoded in a phase:1 rule.

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