Skip to content

Commit 7324e40

Browse files
committed
Fixed a vulnerability with signed requests
1 parent aecd4bf commit 7324e40

File tree

2 files changed

+52
-4
lines changed

2 files changed

+52
-4
lines changed

src/base_facebook.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,11 @@ protected function getUserFromAvailableData() {
529529
if ($signed_request) {
530530
if (array_key_exists('user_id', $signed_request)) {
531531
$user = $signed_request['user_id'];
532+
533+
if($user != $this->getPersistentData('user_id')){
534+
$this->clearAllPersistentData();
535+
}
536+
532537
$this->setPersistentData('user_id', $signed_request['user_id']);
533538
return $user;
534539
}

tests/tests.php

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,20 @@ class PHPSDKTestCase extends PHPUnit_Framework_TestCase {
2222
const MIGRATED_APP_ID = '174236045938435';
2323
const MIGRATED_SECRET = '0073dce2d95c4a5c2922d1827ea0cca6';
2424

25-
const TEST_USER = 499834690;
25+
const TEST_USER = 499834690;
26+
const TEST_USER_2 = 499835484;
2627

2728
private static $kExpiredAccessToken = 'AAABrFmeaJjgBAIshbq5ZBqZBICsmveZCZBi6O4w9HSTkFI73VMtmkL9jLuWsZBZC9QMHvJFtSulZAqonZBRIByzGooCZC8DWr0t1M4BL9FARdQwPWPnIqCiFQ';
2829

29-
private static function kValidSignedRequest() {
30+
private static function kValidSignedRequest($id = self::TEST_USER, $oauth_token = null) {
3031
$facebook = new FBPublic(array(
3132
'appId' => self::APP_ID,
3233
'secret' => self::SECRET,
3334
));
3435
return $facebook->publicMakeSignedRequest(
3536
array(
36-
'user_id' => self::TEST_USER,
37+
'user_id' => $id,
38+
'oauth_token' => $oauth_token
3739
)
3840
);
3941
}
@@ -321,7 +323,6 @@ public function testGetCodeWithMissingCSRFState() {
321323
// intentionally don't set CSRF token at all
322324
$this->assertFalse($facebook->publicGetCode(),
323325
'Expect getCode to fail, CSRF state not sent back.');
324-
325326
}
326327

327328
public function testGetUserFromSignedRequest() {
@@ -335,6 +336,34 @@ public function testGetUserFromSignedRequest() {
335336
'Failed to get user ID from a valid signed request.');
336337
}
337338

339+
public function testSignedRequestRewrite(){
340+
$facebook = new FBRewrite(array(
341+
'appId' => self::APP_ID,
342+
'secret' => self::SECRET,
343+
));
344+
345+
$_REQUEST['signed_request'] = self::kValidSignedRequest(self::TEST_USER, 'Hello sweetie');
346+
347+
$this->assertEquals(self::TEST_USER, $facebook->getUser(),
348+
'Failed to get user ID from a valid signed request.');
349+
350+
$this->assertEquals('Hello sweetie', $facebook->getAccessToken(),
351+
'Failed to get access token from signed request');
352+
353+
$facebook->uncache();
354+
355+
$_REQUEST['signed_request'] = self::kValidSignedRequest(self::TEST_USER_2, 'spoilers');
356+
357+
$this->assertEquals(self::TEST_USER_2, $facebook->getUser(),
358+
'Failed to get user ID from a valid signed request.');
359+
360+
$_REQUEST['signed_request'] = null;
361+
$facebook ->uncacheSignedRequest();
362+
363+
$this->assertNotEquals('Hello sweetie', $facebook->getAccessToken(),
364+
'Failed to clear access token');
365+
}
366+
338367
public function testGetSignedRequestFromCookie() {
339368
$facebook = new FBPublicCookie(array(
340369
'appId' => self::APP_ID,
@@ -1969,6 +1998,20 @@ public function publicGetMetadataCookieName() {
19691998
}
19701999
}
19712000

2001+
class FBRewrite extends Facebook{
2002+
2003+
public function uncacheSignedRequest(){
2004+
$this->signedRequest = null;
2005+
}
2006+
2007+
public function uncache()
2008+
{
2009+
$this->user = null;
2010+
$this->signedRequest = null;
2011+
$this->accessToken = null;
2012+
}
2013+
}
2014+
19722015

19732016
class FBPublicGetAccessTokenFromCode extends TransientFacebook {
19742017
public function publicGetAccessTokenFromCode($code, $redirect_uri = null) {

0 commit comments

Comments
 (0)