Browse Source

[SECURITY] Refactor security hardening code and disable unused stream wrappers

Ensure unwanted enviorment variables are removed from the actual
global environment rather than just the `$_ENV` superglobal variable

Disable stream wrappers, as this is an unexpected feature for most
developers and can be exploited. For instance, `phar://` can be used
to override any class and thus provide code execution (through
`__wakeup` or `__costruct`, for instance). Not a complete solution, as
`php://` can also be abused, but we can't disable it as it gets used
_somewhere_ in our dependencies
v3
Hugo Sales 8 months ago
parent
commit
aef1fac536
Signed by: someonewithpc <hugo@hsal.es> GPG Key ID: 7D0C7EAFC9D835A0
3 changed files with 35 additions and 7 deletions
  1. +4
    -6
      public/index.php
  2. +2
    -1
      src/Core/GNUsocial.php
  3. +29
    -0
      src/Core/Security.php

+ 4
- 6
public/index.php View File

@@ -33,7 +33,7 @@ declare(strict_types = 1);

use App\CacheKernel;
use App\Kernel;
use App\Util\Formatting;
use App\Core\Security;
use Symfony\Component\ErrorHandler\Debug;
use Symfony\Component\HttpFoundation\Request;

@@ -70,11 +70,9 @@ if ('prod' === $kernel->getEnvironment() || isset($_ENV['CONFIG_USE_CACHE_KERNEL
}

$request = Request::createFromGlobals();
$_ENV = array_filter(
$_ENV,
fn (string $key) => Formatting::startsWith($key, ['HTTP', 'APP', 'CONFIG']) && $key !== 'APP_SECRET',
\ARRAY_FILTER_USE_KEY,
);

Security::harden();

$response = $kernel->handle($request);
$response->send();
$kernel->terminate($request, $response);

+ 2
- 1
src/Core/GNUsocial.php View File

@@ -37,8 +37,9 @@ declare(strict_types = 1);
* @author Diogo Cordeiro <diogo@fc.up.pt>
*
* GNU social 3
* @author Diogo Cordeiro <mail@diogo.site>
* @author Hugo Sales <hugo@hsal.es>
* @copyright 2018-2021 Free Software Foundation, Inc http://www.fsf.org
* @copyright 2018-2022 Free Software Foundation, Inc http://www.fsf.org
* @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
*/



+ 29
- 0
src/Core/Security.php View File

@@ -33,7 +33,10 @@ declare(strict_types = 1);
namespace App\Core;

use App\Entity\LocalUser;
use App\Util\Common;
use App\Util\Formatting;
use BadMethodCallException;
use Functional as F;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Security\Core\Security as SymfonySecurity;
use Symfony\Component\Security\Http\Event\LoginFailureEvent;
@@ -76,6 +79,32 @@ class Security implements EventSubscriberInterface //implements AuthenticatorInt
];
}

/**
* Harden running instance. Called once from `index.php`
*/
public static function harden(): void
{
// Remove sensitive information from the
[$_ENV, $to_remove] = F\partition(
$_ENV,
fn ($_, string $key) => Formatting::startsWith($key, ['HTTP', 'APP', 'CONFIG']) && $key !== 'APP_SECRET',
);
F\each($to_remove, fn (mixed $value, string $key) => putenv($key)); // Unset
// Disable stream wrappers, that could be used in things like
// `file_get_contents('https://gnu.org')`. This is done
// because this is a unexpected feature for most developers,
// and some wrappers can be abused. For instance, `phar://`
// can be used to essentially override any class when such a
// file is opened and thus provide code execution to an
// attacker. Not a complete solution, since `file://`,
// `php://` and `glob://` get used _somewhere_, so we can't
// disable them
F\each(
['http', 'https', 'ftp', 'ftps', 'compress.zlib', 'data', 'phar'], // Making this configurable might be a nice feature, but it's tricky because this happens before general initialization
fn (string $protocol) => \stream_wrapper_unregister($protocol)
);;
}

public static function __callStatic(string $name, array $args)
{
if (method_exists(self::$security, $name)) {


Loading…
Cancel
Save