Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$_SERVER var exposes db_name/user/password and salts #474

Open
4 tasks done
kupoback opened this issue Oct 21, 2019 · 8 comments
Open
4 tasks done

$_SERVER var exposes db_name/user/password and salts #474

kupoback opened this issue Oct 21, 2019 · 8 comments
Assignees

Comments

@kupoback
Copy link

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
Copy link
Sponsor Member

QWp6t commented Oct 21, 2019

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
Copy link
Author

Would this also do it for the WP Salts?

@demyxco
Copy link

demyxco commented Oct 22, 2019

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

@kupoback
Copy link
Author

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

@petersafwat-flairs
Copy link

Any Updates on this serious issue?

@austinpray
Copy link
Contributor

There’s code a couple comments up if you want to test it #474 (comment)

@kupoback
Copy link
Author

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
Copy link
Member

Well we need a PR with this change 😄

@QWp6t want to do the honours?

swalkinshaw added a commit that referenced this issue May 29, 2021
Updates to using a custom repository for `Dotenv` instead of the default
which includes `ServerConstAdapter`.

The new custom repository *only* includes `EnvConstAdapter`.

The `$_SERVER` superglobal often gets dumped into logs or into
monitoring services so it's better for security to avoid populating it
with secrets contained in `.env`.
swalkinshaw added a commit that referenced this issue May 29, 2021
Updates to using a custom repository for `Dotenv` instead of the default
which includes `ServerConstAdapter`.

The new custom repository *only* includes `EnvConstAdapter`.

The `$_SERVER` superglobal often gets dumped into logs or into
monitoring services so it's better for security to avoid populating it
with secrets contained in `.env`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants