Skip to content

Commit

Permalink
Read $_COOKIE[session_name()] directly to find session id if sessio…
Browse files Browse the repository at this point in the history
…n is not active (#13)

This will not cover all cases, for example session id will still not be found when the session name is configured in a service somewhere, but the README now covers the case as well by mentioning `addSanitization()`.
  • Loading branch information
spaze authored May 15, 2024
2 parents 62b16eb + 6d713cb commit 462d135
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 3 deletions.
19 changes: 17 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ $this->template->phpinfo = Html::el()->setHtml($this->phpInfo->getHtml());
```

## Sanitization
By default, session id (as returned by `session_id()`) will be sanitized and replaced by `[***]` in the output.
By default, session id (as returned by `session_id()` if session is started, or as stored in `$_COOKIE[session_name()]` if not) will be sanitized and replaced by `[***]` in the output.
This is to prevent some session hijacking attacks that would read the session id from the cookie value reflected in the `phpinfo()` output
(see my [blog post](https://www.michalspacek.com/stealing-session-ids-with-phpinfo-and-how-to-stop-it) describing the attack, `HttpOnly` bypasses, and the solution).
You can disable the sanitization by calling `doNotSanitizeSessionId()` but it's totally not recommended. Do not disable that. Please.
Expand All @@ -39,7 +39,22 @@ $sanitizer = new SensitiveValueSanitizer();
$string = $sanitizer->addSanitization('🍍', '🍌')->sanitize('🍍🍕');
```

The sanitizer will sanitize session id automatically, you can (but shouldn't) disable it with `doNotSanitizeSessionId()`.
The sanitizer will try to determine the session id and sanitize it automatically, you can (but shouldn't) disable it with `doNotSanitizeSessionId()`.

The following values will be automatically used as the session id:
1. `session_id()` output if not `false`
2. `$_COOKIE[session_name()]` if it's a string

However, it is not recommended to rely solely on the automated way, because for example you may set the session name somewhere in a custom service,
and it may not be available for the sanitizer to use. I'd rather suggest you configure the sanitization manually:
```php
$sanitizer->addSanitization($this->sessionHandler->getId(), '[***]'); // where $this->sessionHandler is your custom service for example
```
or
```php
$sanitizer->addSanitization($_COOKIE['MYSESSID'], '[***]'); // where MYSESSID is your session name
```
or something like that.

You can then pass the configured sanitizer to `PhpInfo` class which will then use your configuration for sanitizing the `phpinfo()` output too:
```php
Expand Down
8 changes: 7 additions & 1 deletion src/SensitiveValueSanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ public function sanitize(string $info): string

private function getSessionId(): ?string
{
return session_id() ?: null;
$sessionId = session_id();
if ($sessionId) {
return $sessionId;
} else {
$sessionId = $_COOKIE[session_name()] ?? null;
return is_string($sessionId) ? $sessionId : null;
}
}


Expand Down
29 changes: 29 additions & 0 deletions tests/SensitiveValueSanitizerNoSessionTest.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php
declare(strict_types = 1);

namespace Spaze\PhpInfo;

use Tester\Assert;
use Tester\TestCase;

require __DIR__ . '/bootstrap.php';

class SensitiveValueSanitizerNoSessionTest extends TestCase
{

private const SESSION_ID = 'foobar,baz';


public function testSanitizeSessionNotStarted(): void
{
$_COOKIE[session_name()] = self::SESSION_ID;
$string = (new SensitiveValueSanitizer())->sanitize(sprintf('waldo %s fred', self::SESSION_ID));
Assert::contains('waldo', $string);
Assert::notContains(self::SESSION_ID, $string);
Assert::notContains(urlencode(self::SESSION_ID), $string);
Assert::contains('[***]', $string);
}

}

(new SensitiveValueSanitizerNoSessionTest())->run();

0 comments on commit 462d135

Please sign in to comment.