validateRedirectUri allows for case-insensitivity and substrings?

[dmalan]

We're in the process of deciding exactly what to expect of clients in the way of registration endpoints, and we noticed that validateRedirectUri in OAuth2\Controller\AuthorizeController seems to allow for non-exact matches case-insensitively (assuming require_exact_redirect_uri isn't true). Is this consistent with 3.1.2.2 in the RFC, though, which seems only to allow the below?

If requiring the registration of the complete redirection URI is not possible, the authorization server SHOULD require the registration of the URI scheme, authority, and path (allowing the client to dynamically vary only the query component of the redirection URI when requesting authorization).

In addition to comparing URIs (and thus paths) case-insensitively, validateRedirectUri would seem to allow for substrings, whereby, e.g.,

$registered_uri = "http://example.com/foo";
$inputUri = "http://example.com/foo/bar";

would (incorrectly) be considered matches? Happy to submit a PR, but just wanted to check first if we're drawing the wrong conclusion? Many thanks!

[bshaffer]

@dmalan you are right, we are allowing more than the query component to be altered. A PR would be much appreciated!

[lukestokes]

A security researcher brought this to our attention recently as well.

A registered URL of foobar.com would also match foobar.com.someevildomain.com or even foobar.comeonpeople.someevildomain.com

@dmalan: did you put together a patch for this?

I'm not sure of the best approach, but it seems if the input uri is longer than the registered uri, then the next character (or two) has to be either ?, &, or /?. Does that sound right?

[lukestokes]

Actually, turns out we had require_exact_redirect_uri on all along anyway and the security researcher was looking at a different, older system. Either way, something like this might work, though again, I'm not sure if it's the best approach in the else there:

                if (strlen($inputUri) > strlen($registered_uri)
                    && strcmp(substr($inputUri, 0, strlen($registered_uri)), $registered_uri) === 0
                    && (substr($inputUri, strlen($registered_uri), 1) === '?'
                        || substr($inputUri, strlen($registered_uri), 1) === '&'
                        || substr($inputUri, strlen($registered_uri), 2) === '/?')
                    ) {
                    return true;
                }