Skip to content

Commit 181280a

Browse files
acinaderdplewis
authored andcommitted
Avoid session fixation by regenerating session id on user promotion (#414)
* Regenerate session id when changing the current user to avoid session fixation. * Add to info about session regenerate to the readme. * Add to changelog * add table of contents link. * Update CHANGELOG.md * Update README.md
1 parent 8efe533 commit 181280a

File tree

4 files changed

+78
-3
lines changed

4 files changed

+78
-3
lines changed

CHANGELOG.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
- Adds Parse Server Health Check (#366)
1515
- Adds the ability to upgrade to a revocable session (#368)
1616
- Adds ability to Request Verification Emails (#369)
17-
- Adds the ability to set/save in `ParseConfig` (#371)
17+
- Adds the ability to set/save in `ParseConfig` (#371)
1818
- Adds `ParseLogs` (#370)
1919
- Adds `ParseAudience` (#372)
2020
- Adds jobs to `ParseCloud` (#373)
@@ -40,7 +40,7 @@
4040

4141
- Updates to make the sdk friendly with `phpdoc`
4242
- Added **Getting Started** section to README
43-
- Removed the default server and mount path for `api.parse.com`
43+
- Removed the default server and mount path for `api.parse.com`
4444
- Setup `phpdoc` style enforcing and autodeploy from most recent `master` for our [api ref](http://parseplatform.org/parse-php-sdk/namespaces/Parse.html)
4545
- **jms/serializer** pinned to **1.7.1** for testing as mentioned in #336 (for phpdoc)
4646
- Added **ParsePolygon** type and `polygonContains` to **ParseQuery** (thanks to [Diamond Lewis](https://github.com/dplewis))
@@ -266,4 +266,4 @@
266266
- Updated visibility of `ParseObject::_isDirty` to `protected` (thanks to [Fosco Marotto](https://github.com/gfosco))
267267

268268
### 1.0.0
269-
- Initial release! (thanks to [Fosco Marotto](https://github.com/gfosco))
269+
- Initial release! (thanks to [Fosco Marotto](https://github.com/gfosco))

README.md

+5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ Please note that this documentation contains the latest changes that may as of y
2525
- [Use Declarations](#use-declarations)
2626
- [Parse Objects](#parse-objects)
2727
- [Users](#users)
28+
- [Session Id and Session Fixation](#session-id-and-session-fixation)
2829
- [Verification Emails](#verification-emails)
2930
- [ACLs/Security](#acls)
3031
- [Queries](#queries)
@@ -288,6 +289,10 @@ try {
288289
// Current user
289290
$user = ParseUser::getCurrentUser();
290291
```
292+
#### Session Id and Session Fixation
293+
In an attempt to avoid [session fixation exploits](https://www.owasp.org/index.php/Session_fixation), the PHP SDK will call [`session_regenerate_id()`](http://php.net/manual/en/function.session-regenerate-id.php) when a session's permissions are elevated (since 1.5.0). In practice this means that `session_regenerate_id()` will be called when a session goes from no user to anonymous user or from no user / anonymous user to registered user.
294+
295+
Changing the PHP session id should have no impact on the contents of the session and state should be maintained for a user that was anonymous and becomes registered.
291296

292297
#### Verification Emails
293298

src/Parse/ParseUser.php

+4
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,10 @@ protected function handleSaveResult($makeCurrent = false)
501501
unset($this->serverData['sessionToken']);
502502
}
503503
if ($makeCurrent) {
504+
if (session_id()) {
505+
// see: https://www.owasp.org/index.php/Session_fixation
506+
session_regenerate_id();
507+
}
504508
static::$currentUser = $this;
505509
static::saveCurrentUser();
506510
}
+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php
2+
namespace Parse\Test;
3+
4+
use Parse\ParseClient;
5+
use Parse\ParseUser;
6+
use Parse\ParseSession;
7+
8+
class ParseSessionFixationTest extends \PHPUnit_Framework_TestCase
9+
{
10+
11+
public static function setUpBeforeClass()
12+
{
13+
Helper::clearClass(ParseUser::$parseClassName);
14+
Helper::clearClass(ParseSession::$parseClassName);
15+
ParseUser::logout();
16+
ParseClient::_unsetStorage();
17+
18+
// indicate we should not use cookies
19+
ini_set("session.use_cookies", 0);
20+
// indicate we can use something other than cookies
21+
ini_set("session.use_only_cookies", 0);
22+
// enable transparent sid support, for url based sessions
23+
ini_set("session.use_trans_sid", 1);
24+
// clear cache control for session pages
25+
ini_set("session.cache_limiter", "");
26+
session_start();
27+
Helper::setUp();
28+
}
29+
30+
public function tearDown()
31+
{
32+
Helper::tearDown();
33+
Helper::clearClass(ParseUser::$parseClassName);
34+
Helper::clearClass(ParseSession::$parseClassName);
35+
ParseUser::logout();
36+
}
37+
38+
public static function tearDownAfterClass()
39+
{
40+
session_destroy();
41+
}
42+
43+
public function testCookieIdChangedForAnonymous()
44+
{
45+
ParseClient::getStorage()->set('test', 'hi');
46+
$noUserSessionId = session_id();
47+
$user = ParseUser::loginWithAnonymous();
48+
$anonymousSessionId = session_id();
49+
$this->assertNotEquals($noUserSessionId, $anonymousSessionId);
50+
$this->assertEquals(ParseClient::getStorage()->get('test'), 'hi');
51+
}
52+
53+
public function testCookieIdChangedForAnonymousToRegistered()
54+
{
55+
$user = ParseUser::loginWithAnonymous();
56+
$anonymousSessionId = session_id();
57+
ParseClient::getStorage()->set('test', 'hi');
58+
$user->setUsername('testy');
59+
$user->setPassword('testy');
60+
$user->save();
61+
$user->login('testy', 'testy');
62+
$registeredSessionId = session_id();
63+
$this->assertNotEquals($anonymousSessionId, $registeredSessionId);
64+
$this->assertEquals(ParseClient::getStorage()->get('test'), 'hi');
65+
}
66+
}

0 commit comments

Comments
 (0)