$_SERVER var exposes db_name/user/password and salts

[kupoback]

• [X] I've read the guidelines for Contributing to Roots Projects
• [X] This request isn't a duplicate of an existing issue
• [X] I've read the docs and followed them (if applicable)
• [X] This is not a personal support request that should be posted on the Roots Discourse community

Description

When working on a site, I noticed that if I was to error_log() the $_SERVER variable that the following additional items would be available (name/user/password removed for bug report):

[DB_NAME] => {db_name}
[DB_USER] => {user}
[DB_PASSWORD] => {userspwd}
[WP_ENV] => production
[WP_HOME] => https://domain.test
[WP_SITEURL] => https://wp-boilerplate.test/wp
[AUTH_KEY] => generateme
[SECURE_AUTH_KEY] => generateme
[LOGGED_IN_KEY] => generateme
[NONCE_KEY] => generateme
[AUTH_SALT] => generateme
[SECURE_AUTH_SALT] => generateme
[LOGGED_IN_SALT] => generateme
[NONCE_SALT] => generateme

Steps to reproduce

1. Create a local install, use any environment
2. simply var_dump() or error_log() the $_SERVER var
3. See results

Expected behavior: These sensitive data shouldn't be available via that variable

Actual behavior: These sensitive data shouldn't be able to output

Reproduces how often: 100%

Versions

Bedrock Install: 1.12.8: 2019-09-05 macOS: 10.14.6 laravel/valet 2.3.3

Additional information

Basic Sage 9 with Bedrocks install on local.

[QWp6t]

Update 2020-08-11:

Current syntax looks like this:

$repository = Dotenv\Repository\RepositoryBuilder::createWithNoAdapters()
    ->addAdapter(Dotenv\Repository\Adapter\EnvConstAdapter::class)
    ->immutable()
    ->make();

$dotenv = Dotenv\Dotenv::create($repository, $root_dir);

2019-10-19:

Looks like it would be a pretty straightforward change if we care to do this.

 /**
  * Expose global env() function from oscarotero/env
  */
 Env::init();

 /**
  * Use Dotenv to set required environment variables and load .env file in root
  */
-$dotenv = Dotenv\Dotenv::create($root_dir);
+$dotenv = Dotenv\Dotenv::create($root_dir, null, new Dotenv\Environment\DotenvFactory([
+    new Dotenv\Environment\Adapter\PutenvAdapter(),
+]));
 if (file_exists($root_dir . '/.env')) {
     $dotenv->load();
     $dotenv->required(['WP_HOME', 'WP_SITEURL']);
     if (!env('DATABASE_URL')) {
         $dotenv->required(['DB_NAME', 'DB_USER', 'DB_PASSWORD']);
     }
 }

https://github.com/vlucas/phpdotenv#loader-customization

[kupoback]

Would this also do it for the WP Salts?

[demyxco]

+1 can also confirm when running php -i, the credentials are also shown.

[kupoback]

Tested suggested fix and it removed the SALTS from showing as well.

[petersafwat-flairs]

Any Updates on this serious issue?

[austinpray]

There’s code a couple comments up if you want to test it https://github.com/roots/bedrock/issues/474#issuecomment-544714647

[kupoback]

The above does work, but it should be a change merged into the repo for those that are cloning this for projects. This way they don't have to make a note to copy this solution each time.

[swalkinshaw]

Well we need a PR with this change 😄

@QWp6t want to do the honours?

[SandiyosDev]

Are you all planning on closing solved issues?

[montchr]

@SandiyosDev Before you accuse the maintainers of negligence, I would suggest you verify the truthfulness of your accusation. The issue will be closed automatically once the fix is merged.

598

I would also recommend reading this page:

https://wiki.xxiivv.com/site/discourse.html