roguesupport / oauth-php

The original OAUTH standard implemented in PHP (Historic Record)
MIT License
0 stars 0 forks source link

Serious security issue with multi-user handling #98

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
Having 2 different users (different user ids) getting access tokens through a 
single server using MySQL storage

What is the expected output? What do you see instead?
The user id is not respected by the library - a user may get the access token 
of a different user and use it against the provider successfully

What version of the product are you using? On what operating system?
oauth-php-175

Please provide any additional information below.
In getSecretsForSignature() add this line:
AND oct_usa_id_ref = \'%d\'
to this SQL statement:
$secrets = $this->query_row_assoc('
                    SELECT  ocr_consumer_key        as consumer_key,
                            ocr_consumer_secret     as consumer_secret,
                            oct_token               as token,
                            oct_token_secret        as token_secret,
                            ocr_signature_methods   as signature_methods
                    FROM oauth_consumer_registry
                        JOIN oauth_consumer_token ON oct_ocr_id_ref = ocr_id
                    WHERE ocr_server_uri_host = \'%s\'
                      AND ocr_server_uri_path = LEFT(\'%s\', LENGTH(ocr_server_uri_path))
                      AND (ocr_usa_id_ref = \'%d\' OR ocr_usa_id_ref IS NULL)
                      <ADD IT HERE>
                      AND oct_token_type      = \'access\'
                      AND oct_name            = \'%s\'
                      AND oct_token_ttl       >= NOW()
                    ORDER BY ocr_usa_id_ref DESC, ocr_consumer_secret DESC, LENGTH(ocr_server_uri_path) DESC
                    LIMIT 0,1
                    ', $host, $path, $user_id, $user_id,$name
                    );

Original issue reported on code.google.com by sli...@gmail.com on 22 Feb 2011 at 4:15

GoogleCodeExporter commented 9 years ago
Fixed it: 
https://github.com/sergeychernyshev/oauth-php/commit/31be483d205bac08bd238578bfb
35e360e51ba1b

Same change in patch format attached:

Original comment by sergey.c...@gmail.com on 6 Mar 2011 at 8:00

Attachments:

GoogleCodeExporter commented 9 years ago
Issue 101 has been merged into this issue.

Original comment by scherpenisse on 15 Mar 2011 at 1:05

GoogleCodeExporter commented 9 years ago
Let me know if you have any questions regarding the patch.

Original comment by sergey.c...@gmail.com on 15 Mar 2011 at 1:07

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r189.

Original comment by scherpenisse on 15 Mar 2011 at 1:08

GoogleCodeExporter commented 9 years ago
Thanks for noticing. 

We discovered it ourselves today because we upgraded to the latest oauth-php. 
Apparently we already had fixed it once but did not apply it upstream. Now 
everything should be alright again.

Original comment by scherpenisse on 15 Mar 2011 at 1:09

GoogleCodeExporter commented 9 years ago
Another bug related to this is, if the user is able to somehow change the 
$user_id through HTTP GET potentially, it is still possible to steal all the 
data (i.e. harvest all the ids).

The code should really check user_id in combination with the token and the 
verifier keys provider. At the moment, the user_id with any bogus token and 
verifier keys would still allow access to the service (this has been tested). 
The keys should be signed and compared along with the user_id.

Original comment by ch...@simpleweb.co.uk on 6 Jun 2011 at 1:20