wp-discourse icon indicating copy to clipboard operation
wp-discourse copied to clipboard

Prevent adding bad query string arguments to redirection URL

Open foliovision opened this issue 4 years ago • 1 comments

foliovision avatar Feb 25 '21 05:02 foliovision

This is an fix for the issue I have run into:

Step 1) I open https://forums.example.com/ Step 2) That redirects to https://example.com/?sso={sso value}&sig={sig value} Step 3) Which then redirects to the login page at https://example.com/login?redirect_to=%2F%3Fsso%3D{sso value}%26sig%3D{sig value}%26{sso value}={sig value}

As you can see the {sso value} becomes one of the keys with {sig value} as value in the resulting login URL. It's because of this part of code: https://github.com/discourse/wp-discourse/pull/390/files#diff-b61a5cf65da1b2cdd816f9720c751e11eaa864dc2d972264abdfacf9a7798343L142-L147

Calling add_query_arg() that way results in the first argument being added as key and the second one as value to the current URL obtained using $_SERVER['REQUEST_URI'] internally in that function: https://developer.wordpress.org/reference/functions/add_query_arg/#source

So in my fix I omit that add_query_arg() call as the URL already has ?sso and &sig in it when it reaches that place in the code, so what's the point in adding it again anyway? These query vars are checked right above it: https://github.com/discourse/wp-discourse/pull/390/files#diff-b61a5cf65da1b2cdd816f9720c751e11eaa864dc2d972264abdfacf9a7798343R141

Thanks!

foliovision avatar Feb 25 '21 05:02 foliovision