Decode percent-encoded authority before the followers/sync blocklist#3234
Merged
Decode percent-encoded authority before the followers/sync blocklist#3234
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the /followers/sync REST route’s authority validation by ensuring percent-encoded hosts are decoded before applying the internal-host blocklist, aligning validation with the effective target host for defense-in-depth.
Changes:
- Decode the
authorityvalue prior towp_parse_url()in the/followers/syncvalidate_callbackto catch percent-encoded internal hosts. - Extend the PHPUnit data provider to cover percent-encoded
localhostand bracketed IPv6 literals. - Add a security changelog entry documenting the hardening.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| includes/rest/class-followers-controller.php | Decodes authority before host parsing/blocklist check in the route validator. |
| tests/phpunit/tests/includes/rest/class-test-followers-controller.php | Adds percent-encoded authority variants to the rejection test data provider. |
| .github/changelog/harden-followers-sync-authority-urldecode | Adds a patch-level security changelog entry for the behavior change. |
The /followers/sync authority validate_callback parsed the host straight out of wp_parse_url() without first decoding percent- encoded characters. A value like https://%5B::1%5D/ — bracketed IPv6 literal hidden behind %5B/%5D — would yield a host string of "%5B:" that didn't match any of the localhost / *.local / IP-literal exclusions, so it slipped past the validate_callback. The downstream signer-host check would still reject it (the signer's keyId never decodes to %5b:), so this isn't an exploitable SSRF on its own — but it's defense-in-depth that lines up the validator's decisions with the real host the request would target. Run urldecode() on the input before parsing so encoded forms are checked against the same blocklist as their decoded equivalents. Cover three encoded variants in the rejected_authority data provider.
urldecode() also turns '+' into a space, which would corrupt otherwise-valid reg-name hosts that legitimately contain '+' (e.g. test/staging hosts like example+test.com). rawurldecode() performs the percent-decoding we want without that querystring '+' semantic. Also drop the trailing slash from the docblock example so it matches the route's `^https?://[^/]+$` pattern.
The validate_callback now rawurldecodes $param before extracting the host, but get_partial_followers() still parsed the raw authority. That meant a percent-encoded but otherwise-valid host (e.g. "https://%70eer.example") could pass validation yet fail the authority-mismatch check downstream because the signer's keyId would decode to a different string than the raw asked_host. Apply the same rawurldecode() in get_partial_followers() so the two sites agree on the canonical form.
pfefferle
force-pushed
the
harden/followers-sync-authority-urldecode
branch
from
99a585d to
1dc7280
Compare
The previous fix only canonicalised $authority on the signer-host comparison. The Followers::get_by_authority() meta query and the response `id` still received the raw, possibly percent-encoded value. A request with `https://%70eer.example` would pass the match (both sides decode to `peer.example`) but the LIKE meta query would search for the literal `%70eer.example` and never hit stored inbox URLs like `https://peer.example/inbox`, returning an empty follower set. Decode once at the top of the handler and use that canonical string throughout — match check, follower query, and response `id`. Adds a regression test that seeds a follower on `peer.example`, asks with `https://%70eer.example`, and asserts the alice follower is returned.
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
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.
Proposed changes:
/followers/syncauthorityvalidate_callbackparsed the host straight out ofwp_parse_url()without first decoding percent-encoded characters. A value likehttps://%5B::1%5D/— bracketed IPv6 literal hidden behind%5B/%5D— yielded a host string of%5B:that didn't match any of thelocalhost/*.local/ IP-literal exclusions, so it slipped past the validate_callback.get_partial_followers()would still reject the request (the signer's keyId never decodes to%5b:), so this isn't an exploitable SSRF on its own. The fix lines up the validator's decisions with the real host the request would target — defense-in-depth.urldecode()on the input before parsing so encoded forms are checked against the same blocklist as their decoded equivalents.%6Cocalhost,%5B::1%5D,%5B::ffff:10.0.0.1%5D.This is opt-in defense-in-depth — the touched path only fires when FEP-8fcf
/followers/syncis hit by a remote peer.Other information:
Testing instructions:
/followers/syncrequest withauthority=https://%5B::1%5D/. The validate_callback should reject it before signature verification, returning400 rest_invalid_param.authority=https://%6Cocalhost(encodedlocalhost) — also rejected at the validator.Changelog entry
Changelog Entry Details
Significance
Type
Message
Decoded percent-encoded forms in the follower sync authority before the safety check.