Skip to content

Commit

Permalink
CVE-2022-24444 Update expiry on session destroy
Browse files Browse the repository at this point in the history
  • Loading branch information
emteknetnz committed Jun 27, 2022
1 parent bc35403 commit 1709ed9
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
34 changes: 31 additions & 3 deletions src/Store/DatabaseStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,21 @@
use SilverStripe\Core\Convert;
use SilverStripe\HybridSessions\Store\BaseStore;
use Exception;
use SilverStripe\HybridSessions\HybridSessionDataObject;

class DatabaseStore extends BaseStore
{
/**
* Hashing algorithm used to encrypt $session_id (PHPSESSID)
* Ensure that HybridSessionDataObject.SessionID is wide enough to accomodate the hash
*/
private const HASH_ALGO = 'sha256';

/**
* @var ?bool
*/
private $hashAlgoAvailable = null;

/**
* Determine if the DB is ready to use.
*
Expand Down Expand Up @@ -55,7 +67,7 @@ public function read($session_id)
$query = sprintf(
'SELECT "Data" FROM "HybridSessionDataObject"
WHERE "SessionID" = \'%s\' AND "Expiry" >= %s',
Convert::raw2sql($session_id),
Convert::raw2sql($this->encryptSessionID($session_id)),
$this->getNow()
);

Expand All @@ -81,7 +93,7 @@ public function write($session_id, $session_data)
'INSERT INTO "HybridSessionDataObject" ("SessionID", "Expiry", "Data")
VALUES (\'%1$s\', %2$u, \'%3$s\')
ON DUPLICATE KEY UPDATE "Expiry" = %2$u, "Data" = \'%3$s\'',
Convert::raw2sql($session_id),
Convert::raw2sql($this->encryptSessionID($session_id)),
$expiry,
Convert::raw2sql(static::binaryDataJsonEncode($session_data))
));
Expand All @@ -92,7 +104,15 @@ public function write($session_id, $session_data)
#[\ReturnTypeWillChange]
public function destroy($session_id)
{
// NOP
if (!$this->isDatabaseReady()) {
return false;
}
DB::query(sprintf(
'DELETE FROM "HybridSessionDataObject"
WHERE "SessionID" = \'%s\'',
Convert::raw2sql($this->encryptSessionID($session_id))
));
return true;
}

#[\ReturnTypeWillChange]
Expand Down Expand Up @@ -150,4 +170,12 @@ public static function binaryDataJsonDecode($text)

return base64_decode($struct[1] ?? '');
}

private function encryptSessionID(string $sessionID): string
{
if (is_null($this->hashAlgoAvailable)) {
$this->hashAlgoAvailable = in_array(self::HASH_ALGO, hash_algos());
}
return $this->hashAlgoAvailable ? hash(self::HASH_ALGO, $sessionID) : $sessionID;
}
}
11 changes: 11 additions & 0 deletions tests/DatabaseStoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,15 @@ public function testDataCodecIntegrity()
$this->assertEquals($data, DatabaseStore::binaryDataJsonDecode(DatabaseStore::binaryDataJsonEncode($data)));
}
}

public function testWriteReadAndDestroy()
{
$sessionID = 'session-id-123456789';
$data = 'a-blob-of-session-data';
$store = $this->getStore();
$store->write($sessionID, $data);
$this->assertSame($data, $store->read($sessionID));
$store->destroy($sessionID);
$this->assertNull($store->read($sessionID));
}
}

0 comments on commit 1709ed9

Please sign in to comment.