Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Wrong third parameters for the getSelfRoutedURLNoQuery in Utils.php #467

Closed
arthurju opened this issue Feb 18, 2021 · 8 comments
Closed

Comments

@arthurju
Copy link

Hello,

I'm facing a bug on Utils.php

The code :
$pos = strpos($selfRoutedURLNoQuery, "?");
if ($pos !== false) {
$selfRoutedURLNoQuery = substr($selfRoutedURLNoQuery, 0, $pos-1);
}

I'm using this bundle on a Nginx web server routing 2 symfony applications.
On my first app I log my users with SSO and the remote IDP return to the URL I defined => myproject.com/app/saml/acs.
The return request go through the function getSelfRoutedURLNoQuery and at the line 645 the function check if there is a "?" in the request (which in my conf NGINX does).
But if a "?" is found, the route is goes through substr and on my side there is a mistake with the third parameter $pos-1 which remove one char before the ?. So instead of have a route like myproject.com/app/saml/acs I have a route like that : myproject.com/app/saml/ac
In my opinion is a bug and someone should fix into $selfRoutedURLNoQuery = substr($selfRoutedURLNoQuery, 0, $pos);

@pitbulk
Copy link
Contributor

pitbulk commented Feb 18, 2021

This method tries to get a substring from selfRoutedURLNoQuery if an ? appears

If you have

myproject.com/app/saml/acs?xxxxx

then it calculates the position of ?, and then get the substring before the ?, so

myproject.com/app/saml/acs

Can you add a debug at this method and print before and after the line of the

$selfRoutedURLNoQuery = substr($selfRoutedURLNoQuery, 0, $pos-1);

what values has $pos and $selfRoutedURLNoQuery?

@arthurju
Copy link
Author

Indeed I have an uri like myproject.com/app/saml/acs?xxxxx.
The strpos ($pos) return 34 for my 60 length uri.
strpos return the position of the "?" substr start at 0 and must finish juste before the "?". So it must finish at the position 34 (at the $pos position). I don't understand why there is a minus one at the $pos which remove the last char before the ?
At the end the var $selfRoutedURLNoQuery return myproject.com/app/saml/ac and not myproject.com/app/saml/acs cause of the minus one after the $pos.

@pitbulk
Copy link
Contributor

pitbulk commented Feb 19, 2021

Thanks for reporting this.
Feel free to update the unit test with your case

@dovbeta
Copy link

dovbeta commented Feb 25, 2021

Thanks @pitbulk for this fix. Coult it be added to v3 too?

@pitbulk
Copy link
Contributor

pitbulk commented Feb 25, 2021

It is at 3.6.1 branch already: https://github.com/onelogin/php-saml/commits/3.6.1

@Joeb454
Copy link

Joeb454 commented Mar 2, 2021

@pitbulk we're impacted by this issue too, when do you see 3.6.1 being available as a release?

@pitbulk
Copy link
Contributor

pitbulk commented Mar 2, 2021

Versions 2.19.1, 3.6.1 and 4.0.0 released today.

Let me know if you experience any issues.

@Joeb454
Copy link

Joeb454 commented Mar 2, 2021

Amazing, thank you!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants