Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Steam] Authorization domain spoof check #817

Merged
merged 7 commits into from
Mar 28, 2022

Conversation

strd0x
Copy link
Contributor

@strd0x strd0x commented Mar 15, 2022

This fixes broken auth vulnerability.

Exploit:
Generate url for authorization through this package:
Original link: https://steamcommunity.com/openid/login?openid.ns=http://specs.openid.net/auth/2.0&openid.mode=checkid_setup&openid.return_to=http://smth.local&openid.realm=http://smth.local&openid.identity=http://specs.openid.net/auth/2.0/identifier_select&openid.claimed_id=http://specs.openid.net/auth/2.0/identifier_select

And when Steam takes us back, we'll get url containing our login details. Send this data to the domain of some popular platform, for example, BitSkins

Original returned url: http://smth.local/?openid.ns=http://specs.openid.net/auth/2.0&openid.mode=id_res&openid.op_endpoint=https://steamcommunity.com/openid/login&openid.claimed_id=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.identity=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.return_to=http://smth.local&openid.response_nonce=2022-03-15T21:41:08ZezdPkwX/lqSJVEfBwgG4UJZGwGo=&openid.assoc_handle=1234567890&openid.signed=signed,op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle&openid.sig=Z7u/5KZTErhVOTVqO7o+7Pt4rYQ=

We'll change it for: http://bitskins.com/?openid.ns=http://specs.openid.net/auth/2.0&openid.mode=id_res&openid.op_endpoint=https://steamcommunity.com/openid/login&openid.claimed_id=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.identity=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.return_to=http://smth.local&openid.response_nonce=2022-03-15T21:41:08ZezdPkwX/lqSJVEfBwgG4UJZGwGo=&openid.assoc_handle=1234567890&openid.signed=signed,op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle&openid.sig=Z7u/5KZTErhVOTVqO7o+7Pt4rYQ=

OpenID will check the link and send a validate request, but we didnt change data, so validate will work correctly and authorize the user on another site, although he might not even suspect this, since he was authorized on the site, for example, with articles CS: GO

@strd0x strd0x force-pushed the steam-domain-validation branch 2 times, most recently from 3baf467 to 39e26a7 Compare March 15, 2022 21:25
@strd0x strd0x changed the title Add authorization domain validation [Steam] Authorization domain spoof check Mar 15, 2022
@EdgarSedov
Copy link

@atymic @lucasmichot
Can u take a look, please? We would like to use original package instead of our fork

@lucasmichot
Copy link
Member

@strd0x can you provide a link to the doc (not behind login wall) ?

@strd0x
Copy link
Contributor Author

strd0x commented Mar 16, 2022

Hello!
For example, we have:

  1. CS: GO Articles website
  2. Large Steam market website

Let's imagine that website 1 was hacked and the hackers started killing return requests from Steam, while saving the return parameters somewhere, not passing OpenID validation, and so on. Next, the hackers take this url and direct it to website 2 - they get full access to the victim's account on this site.

Saved url: smth.local/?openid.ns=http://specs.openid.net/auth/2.0&openid.mode=id_res&openid.op_endpoint=https://steamcommunity.com/openid/login&openid.claimed_id=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.identity=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.return_to=http://smth.local&openid.response_nonce=2022-03-15T21:41:08ZezdPkwX/lqSJVEfBwgG4UJZGwGo=&openid.assoc_handle=1234567890&openid.signed=signed,op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle&openid.sig=Z7u/5KZTErhVOTVqO7o+7Pt4rYQ=

Change it: very-large-market-steam.com/?openid.ns=http://specs.openid.net/auth/2.0&openid.mode=id_res&openid.op_endpoint=https://steamcommunity.com/openid/login&openid.claimed_id=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.identity=https://steamcommunity.com/openid/id/7656119XXXXXXXXXX&openid.return_to=http://smth.local&openid.response_nonce=2022-03-15T21:41:08ZezdPkwX/lqSJVEfBwgG4UJZGwGo=&openid.assoc_handle=1234567890&openid.signed=signed,op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle&openid.sig=Z7u/5KZTErhVOTVqO7o+7Pt4rYQ=

Check the parameters of both requests above. Here is no difference, but we had changed domain.

This is real case, that had tested on the real website.

{
$allowedHosts = $this->getConfig('allowed_hosts', []);

return count($allowedHosts) === 0 || in_array(parse_url($url, PHP_URL_HOST), $allowedHosts, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be backwards compatible right, as it will return true if no domains are configured?
Maybe add some more comment to readme on why allowed hosts is important?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is why the setting is ignored if it is missing.
I think it could be added as a description that this setting only allows authorization from certain domains to protect against user authorization spoofing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atymic Should we add this to the readme?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please, not much point in adding this security feature if it defaults to off and isn't documented

Copy link
Contributor Author

@strd0x strd0x Mar 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atymic I've added this for readme

@EdgarSedov
Copy link

Guys, what do we need to do to get this merged? We would like to use updated version in current project

@EdgarSedov
Copy link

EdgarSedov commented Mar 28, 2022

Anyone? :'(
@atymic @lucasmichot Please tell us what changes do you need to get this merged

@atymic atymic merged commit 79350e2 into SocialiteProviders:master Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants