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.

Sign up for GitHub

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

Jump to bottom

False positive while checking default wordpress config #77

Closed
BeLove opened this issue Mar 6, 2018 · 4 comments
Closed

False positive while checking default wordpress config #77

BeLove opened this issue Mar 6, 2018 · 4 comments

Comments

@BeLove
Copy link

BeLove commented Mar 6, 2018

Here is default nginx config for WP: https://codex.wordpress.org/Nginx
And here is a line:

rewrite /wp-admin$ $scheme://$host$uri/ permanent;

Gixy warns us with next issue:

>> Problem: [http_splitting] Possible HTTP-Splitting vulnerability. Description: Using variables that can contain "\n" or "\r" may lead to http injection. ... Reason: At least variable "$uri" can contain "\n"

But this is false positive as Rewrite directive isn't vulnerable to CRLF.
@BeLove BeLove closed this as completed Mar 6, 2018
@buglloc
Copy link
Member

buglloc commented Mar 6, 2018

Nope, vulnerable ;-)
Config:

server {
    listen       80;
    server_name  _;

    rewrite /wp-admin$ $scheme://$host$uri/ permanent;
}

Test:

$ http -v http://localhost/%0d%0ax-injection:/wp-admin
GET /%0d%0ax-injection:/wp-admin HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Host: localhost
User-Agent: HTTPie/0.9.9



HTTP/1.1 301 Moved Permanently
Connection: keep-alive
Content-Length: 185
Content-Type: text/html
Date: Tue, 06 Mar 2018 17:45:42 GMT
Location: http://localhost/
Server: nginx/1.12.2
x-injection: /wp-admin/

<html>
<head><title>301 Moved Permanently</title></head>
<body bgcolor="white">
<center><h1>301 Moved Permanently</h1></center>
<hr><center>nginx/1.12.2</center>
</body>
</html>

@BeLove
Copy link
Author

BeLove commented Mar 6, 2018

Yup, that is why I closed it. Just notified wp team

@janw-me
Copy link

janw-me commented Sep 12, 2019

This line is still on the support page. What should be the correct syntax?

@mitogh
Copy link

mitogh commented Dec 24, 2019

In order to fix this make sure to update from:

    rewrite /wp-admin$ $scheme://$host$uri/ permanent;

Into:

    rewrite /wp-admin$ $scheme://$host$request_uri/ permanent;

So the only change is $host$uri into $host$request_uri this change now is reflected on: https://wordpress.org/support/article/nginx/

    if (!-e $request_filename) {
        rewrite /wp-admin$ $scheme://$host$request_uri/ permanent;
        rewrite ^(/[^/]+)?(/wp-.*) $2 last;
        rewrite ^(/[^/]+)?(/.*\.php) $2 last;
    }

@zak-wearecore zak-wearecore mentioned this issue Oct 21, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@BeLove @janw-me @mitogh @buglloc