Consistently unslash superglobals before use - #100
Open
miyanialkesh7 wants to merge 1 commit into
Open
Conversation
mustafauysal
requested changes
Aug 24, 2026
| // Use a generic error message to ensure user ids can't be sniffed | ||
| $user_id = (int) $_GET['user_id']; //phpcs:ignore WordPress.Security.NonceVerification.Recommended | ||
| $token = $_GET['token']; //phpcs:ignore | ||
| $token = sanitize_text_field( wp_unslash( $_GET['token'] ) ); //phpcs:ignore WordPress.Security.NonceVerification.Recommended |
Member
There was a problem hiding this comment.
Please avoid passing the magic-login token through sanitize_text_field(). The token is an opaque value that is hashed and compared exactly. Sanitizing it provides no additional security in this code path and may modify tokens customized through the magic_login_create_user_token filter. wp_unslash( $_GET['token'] ) should be sufficient here.
| == Changelog == | ||
|
|
||
| = 2.8.2 (Aug 15, 2026) = | ||
| * [Improved] Consistently unslash $_GET/$_POST/$_REQUEST values (redirect_to, login token, token interval) before sanitizing/using them, for defense-in-depth. |
Member
There was a problem hiding this comment.
Please remove the version bump and changelog entry from this PR. Release metadata will be handled separately so it can be coordinated with other pending changes.
Just add changelog items as comment.
$_GET/$_POST/$_REQUEST reads for the login redirect target, the magic-link token, and the settings token interval were used without wp_unslash() first in a few spots. Output was already escaped downstream in most cases, so this is defense-in-depth hardening rather than a fix for an exploitable issue, and brings these call sites in line with the wp_unslash() + sanitize pattern already used everywhere else in the codebase. The magic-link token itself is left unsanitized after wp_unslash(): it's an opaque value that's hashed and compared exactly, so sanitize_text_field() adds no security and risked mangling tokens customized via the magic_login_create_user_token filter.
miyanialkesh7
force-pushed
the
fix/security-input-sanitization-hardening
branch
from
August 25, 2026 06:12
fced613 to
ce09b5c
Compare
Author
|
@mustafauysal Addressed both review comments:
Re-ran |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wp_unslash()before sanitizing/using$_GET/$_POST/$_REQUESTvalues in 7 spots (redirect_to handling in LoginManager, CodeLogin, shortcode; the magic-link token; the settings token_interval switch).wp_unslash()— it's an opaque value that's hashed and compared exactly, sosanitize_text_field()adds no security and risked mangling tokens customized via themagic_login_create_user_tokenfilter.Why
Found during a security audit pass: these call sites were the only ones in the codebase reading superglobals without
wp_unslash()first.Test Plan
php -lon all changed files — no syntax errorsvendor/bin/phpcs --standard=phpcs.xmlon changed files — 0 errors/warningsvendor/bin/phpunit— 8/8 passingwp-login.php?redirect_to=...renders the hidden field correctly (unslashed + escaped)