fkoyer opened a new pull request, #41:
URL: https://github.com/apache/spamassassin/pull/41
Replaces the hand-rolled parsing in _parse_uri and _extract_embedded_uri
with the URI class. Extraction becomes a loop over parsed query parameters
rather than regexes run against the raw querystring, which removes a class of
bug rather than patching instances of it:
- An encoded separator inside a parameter value no longer ends it. The
query is split while still encoded and each value decoded afterwards, so
?url=evil.com%2Fp%3Fx%3D1%26tid%3Dgood.com keeps its whole target.
- Whether url_redirector_params captures greedily stops mattering, since
splitting the query replaces capturing it. The setting is now matched against
one "name=value" at a time, so a greedy capture can no longer run past the end
of the value it belongs to. Its capture group is still honoured where it has
one, which lets a regex pull the target out of a value that wraps it, as in
"u=12345|https://example.com/";; with no capture group the whole value is the
candidate.
- An embedded target that is itself percent-encoded is found without a
second pattern to match the encoded form.
Extraction runs in three stages: a parameter whose name matches
url_redirector_params, then any parameter whose value looks like a URI, then a
target embedded in the path. The first stage keeps a configured name winning
over a decoy such as a logo or a return URL appearing earlier in the
querystring; the second finds targets under names no list anticipates.
Two fixes fall out of the rewrite:
- A fragment is stripped before fetching, on both sides of the
self-redirect comparison. RFC 7231 lets a Location carry one, so a redirector
that echoed a fragment defeated the comparison and the destination was walked
as though it were a new hop.
- A URI is now searched for an embedded one on its own merits: the scheme,
and whether there is a path or query to search. The wrapper's host is not
consulted, since an embedded URI is never taken from it -- requiring a dot
there meant https://localhost/?url=https://evil.com/x and any other dotless
intranet host had their target ignored.
_walk_redirects parses each URI once and passes the object down, so a chain
of three hops parses four URIs rather than seven.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
