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

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 committed May 15, 2024
1 parent 62b16eb commit 6d713cb
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 6d713cb

Please sign in to comment.