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

Use SCRIPT_NAME instead of REQUEST_URI to check path #589

Merged
merged 2 commits into from
May 15, 2024

Conversation

2ndkauboy
Copy link
Member

The script is currently checking if the REQUEST_URI is containing wp-comments-post.php, the default script to handle the submission of a comment. Some security plugins have options to rename this file to disguise that WordPress is used.

With this fix, the SCRIPT_NAME is used instead. Since many security plugins do use rewrite rules, while the REQUEST_URI value is changed, the SCRIPT_NAME value stays the same. Therefore, the condition would still recognize if a comment was submitted.

Fixes #585

The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefore the condition would
still recognize if a comment was submitted.

Fixes #585
@2ndkauboy 2ndkauboy requested review from Zodiac1978 and stklcode May 6, 2024 19:57
@2ndkauboy 2ndkauboy force-pushed the fix/585-use-script-name-for-request-path branch from 230bbcd to e861fcd Compare May 7, 2024 19:06
@2ndkauboy 2ndkauboy closed this May 7, 2024
@2ndkauboy 2ndkauboy force-pushed the fix/585-use-script-name-for-request-path branch from e861fcd to cb75530 Compare May 7, 2024 19:38
@2ndkauboy
Copy link
Member Author

Somehow, I accidentally closed the PR 🤔

@2ndkauboy 2ndkauboy reopened this May 7, 2024
@2ndkauboy 2ndkauboy requested a review from stklcode May 7, 2024 20:10
@2ndkauboy 2ndkauboy force-pushed the fix/585-use-script-name-for-request-path branch from f68532c to 899df20 Compare May 7, 2024 20:16
@stklcode stklcode force-pushed the fix/585-use-script-name-for-request-path branch 6 times, most recently from b4ed229 to 899df20 Compare May 13, 2024 19:19
Copy link

sonarcloud bot commented May 13, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Zodiac1978
Copy link
Member

Possible alternative approach:

johndarrel/hide-my-wp#2 (comment)

@2ndkauboy
Copy link
Member Author

@Zodiac1978 I don't think it's an alternative approach, it's just another step. Currently, the condition is the following:

strpos( $request_path, 'wp-comments-post.php' ) !== false

The $request_path used to be the REQUEST_URI, which would change, if a security plugin "changes this script name" using a rewrite rule. By using SCRIPT_NAME, we make sure we get the script name of the actual script that handles the request.

This is only affecting the left side of the condition and the first parameter $request_path. With home_url(), we could replace the second parameter, the static wp-comments-post.php string. But this would still not solve the issue. Replacing the first parameter with home_url(), the condition would most probably always be true, right?

Since this PR resolves the issue, I would not introduce a new function call (twice), we probably don't need.

@stklcode
Copy link
Contributor

home_url(…) should equal REQUEST_URI. But I don’t see much of a benefit over the (cheaper) SCRIPT_NAME comparison.

@2ndkauboy
Copy link
Member Author

The home_url() path is not looking at any $_SERVER values at all. All it does (in this case) is prepending the path with a slash. So calling home_url('wp-comments-post.php', 'relative') does always return /wp-comments-post.php, unless there is a filter hooking into home_url, which Hide My WP does not seem to do. So this is no solution.

@Zodiac1978 are you also OK to merge this then?

@Zodiac1978
Copy link
Member

are you also OK to merge this then?

I just asked the author and if his approach is not smart, let's do it better.

From my understanding, it would still solve the issue with his plugin, so yes, I'm fine with PR!

Copy link
Member

@Zodiac1978 Zodiac1978 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@stklcode stklcode merged commit 0b61f08 into master May 15, 2024
17 of 31 checks passed
@2ndkauboy 2ndkauboy deleted the fix/585-use-script-name-for-request-path branch May 16, 2024 07:49
stklcode added a commit that referenced this pull request May 16, 2024
The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefor the condition would
still recognize if a comment was submitted.

Original fix by @2ndkauboy in #589, adapted to v3.
stklcode added a commit that referenced this pull request May 16, 2024
The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefor the condition would
still recognize if a comment was submitted.

Original fix by @2ndkauboy in #589, adapted to v3.
stklcode added a commit that referenced this pull request May 16, 2024
The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefor the condition would
still recognize if a comment was submitted.

Original fix by @2ndkauboy in #589, adapted to v3.
@stklcode stklcode added this to the 2.11.7 milestone May 16, 2024
stklcode added a commit that referenced this pull request May 22, 2024
The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefor the condition would
still recognize if a comment was submitted.

Original fix by @2ndkauboy in #589, adapted to v3.
2ndkauboy pushed a commit that referenced this pull request Nov 1, 2024
The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefor the condition would
still recognize if a comment was submitted.

Original fix by @2ndkauboy in #589, adapted to v3.
stklcode added a commit that referenced this pull request Nov 1, 2024
The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefor the condition would
still recognize if a comment was submitted.

Original fix by @2ndkauboy in #589, adapted to v3.
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

Successfully merging this pull request may close these issues.

Hide My WP Ghost - Security Obfuscation
3 participants