Open Redirect

[Belippo]

During analysis of the SPID authentication flow implemented via the Shibboleth plugin, I identified a possible open redirect issue caused by the propagation of a user-controlled target parameter through the authentication

Steps to Reproduce:

1. At first visit this url http://target.com/Shibboleth.sso/Login?target=
2. Then use any url in target query parameter.
3. your link will be look like:
   http://target.com/Shibboleth.sso/Login?target=https://{site}.com
4. login your account and you will redirect to the url.

[jrchamp]

Thank you @Belippo! Here's what I'm thinking:

1. Use wp_validate_redirect() to make sure the redirect is in the allowed list.
2. Add a filter for allowed_redirect_hosts so that the redirects work on multisite.

add_filter( 'allowed_redirect_hosts', function( $hosts, $host ) {
    // If the host is already in the list, return early
    if ( in_array( $host, $hosts, true ) ) {
        return $hosts;
    }

    // Check if this host belongs to any site in our Multisite network.
    if ( get_site_by_path( $host, '/' ) ) {
        $hosts[] = $host;
    }

    return $hosts;
}, 10, 2 );

I'll probably change it so that the return value from the get site function is used to add the value to the hosts array. And the default redirect that is passed to the validate function will probably be the site URL / home URL. Maybe also good to add an "is Multisite" check to avoid unnecessary lookups. I do wonder if caching would be beneficial to mitigate spam on large Multisite environments.

[Belippo]

Thank you for your reply.

Yes, at the moment there is no validation for the user‑controllable parameter, so I agree that the proper approach is, as you suggested, to implement functions that ensure submitted URLs are included in the allowed list. A stronger mitigation, which is not uncommon, would be to avoid passing the target URL directly to the redirector and instead pass an index referencing the list (see PortSwigger reference).
Since you have confirmed the issue, I am considering requesting a CVE ID from a CNA.

[jrchamp]

Opening a CVE for this plug-in sounds good, but please note that /Shibboleth.sso/* is the Shibboleth SP software and not this plug-in. There does appear to be an open redirect in this plug-in, but it's earlier in the authentication flow. I don't have a computer right now, so it'll still be a bit before I can fix it myself.

[Jefhumbe]

Thank you, @Belippo.

So the vulnerability is in this WordPress plugin and not in the Shibboleth SP software, correct? @jrchamp

[michaelryanmcneill]

The exploit you are describing is a behavior of the Shibboleth SP and not the plugin. As @jrchamp mentioned, there is an open redirect issue, but it happens earlier in the authentication flow than this and is not demonstrated by this issue.

[michaelryanmcneill]

For more information on how to address the specific exploit you've described here, review this article: https://support.aaf.edu.au/support/solutions/articles/19000130194-is-your-sp-vulnerable-to-being-used-as-an-open-redirector.

[jrchamp]

@Belippo @Jefhumbe If any of you have a chance to look at #112, please let me know if it causes any issues. The code makes sense to me, but I just set up my computer today, so I don't have a complex test environment to verify with just yet. Thank you! ❤️

[jrchamp]

Now that #112 is merged and 2.5.3 is released, I'm going to mark this as resolved. I had considered adding documentation for the SP issue to the plugin, but that isn't sustainable. The best practices for the official SP software should be (and are) provided by the SP documentation.

In case anyone checks here, I will note it here:

Make sure that the redirectLimit="exact" attribute is part of your <Sessions> element in the main Shibboleth configuration file. That is the default configuration on fresh installs.