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

security issue: env.php must be writeable in production #4955

Closed
ludwig-gramberg opened this issue Jun 10, 2016 · 35 comments
Closed

security issue: env.php must be writeable in production #4955

ludwig-gramberg opened this issue Jun 10, 2016 · 35 comments
Assignees
Labels
bug report Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Progress: needs update Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@ludwig-gramberg
Copy link

I have an issue with Magento expecting the env.php file to be writeable in production, potentially an attacker who gains code-execution in the webserver could then manipulate this file to execute arbitrary code

in production I want to be able lock down this file so nobody can manipulate it, after all this is the production mode, so no code-generation is necessary (and if you must store parts of the config in a writeable way inside a file, don't use a php-file, use xml or ini)

@ludwig-gramberg ludwig-gramberg changed the title security issue: env.php be writeable in production security issue: env.php must be writeable in production Jun 10, 2016
@kandy
Copy link
Contributor

kandy commented Jun 10, 2016

Can you point on functionality that required writeable env.php in production?

@ludwig-gramberg
Copy link
Author

its the cache, as soon as a cache option is disabled, it attempts to rewrite the file:

#2 /srv/magento/vendor/magento/framework/App/DeploymentConfig/Writer.php(113): Magento\\Framework\\Filesystem\\Directory\\Write->writeFile('env.php', '<?php\\nreturn ar...')
#3 /srv/magento/vendor/magento/framework/App/Cache/State.php(101): Magento\\Framework\\App\\DeploymentConfig\\Writer->saveConfig(Array)
#4 /srv/magento/vendor/magento/framework/App/Cache/Manager.php(74): Magento\\Framework\\App\\Cache\\State->persist()
#5 /srv/magento/vendor/magento/framework/App/ObjectManager/Environment/Compiled. in /srv/magento/vendor/magento/framework/Filesystem/Directory/Write.php on line 53

version is 2.0.7

@ludwig-gramberg
Copy link
Author

also i think if you disable/enable any cache it rewrites the file

@kandy
Copy link
Contributor

kandy commented Jun 10, 2016

Don't think that you need disable the cache on production.

@ludwig-gramberg
Copy link
Author

it just breaks functionality or security so its bad design

@kandy
Copy link
Contributor

kandy commented Jun 10, 2016

Magento can be used in different environment. In developer mode, you can clean cache because in this case you don't need maximum security, in production mode, you cannot disable cache with maximum security settings. So no security issue there.

@ludwig-gramberg
Copy link
Author

but it wont work with any cache disabled, if I modify my env.php and disable one of the caches the app is broken (it wont load anything being stuck on trying to rewrite the file), also the admin-interface suggest to disable caches, this would also result in a failed write attempt

another aspect is, that I'm not sure which other parts of the code for whatever reasons might attempt to rewrite this file... if the app was designed to be able to write this file in either of the modes, then something else might break my shop at any time, or an update might

so my point is that either the app expects the file to writeable at all times (which is bad for security as long as this is a a php-file), or it should take into account that the file might not be writeable. both are not the case.

@ludwig-gramberg
Copy link
Author

I can only guess as to why this file was chosen as a php-file, might as well be an ini-file using http://php.net/manual/en/function.parse-ini-file.php keeping the dependency to read this cfg to a minimum, either way i think its a bad practice to mix "static"-cfg-values given by the environment and settings of the app (i.e. the cache being enabled/disabled)

@ludwig-gramberg
Copy link
Author

it really doesn't work, now I can't even upgrade anymore in production:

checkInstallationFilePermissions in setup/src/Magento/Setup/Model/Installer.php

is not letting me pass... it doesn't even need to write any file... it just checks for it...

@ludwig-gramberg
Copy link
Author

I wrote my own upgrade console command with a custom InstallerFactory which injects a custom FilePermission class, so it upgrades withouth bothering to check the permissions first (which I don't need in production).

but this only proofs, that yes magento expects to write these files and wont work without being able to write them.

@wert2all wert2all self-assigned this Jun 23, 2016
@wert2all wert2all added the TECH label Jun 23, 2016
@wert2all wert2all removed their assignment Jun 23, 2016
@buskamuza
Copy link
Contributor

Internal ticket MAGETWO-55424 to consider non-executable format for configuration files.

@veloraven veloraven added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report labels Aug 18, 2016
@buskamuza buskamuza removed their assignment Aug 18, 2016
@ludwig-gramberg
Copy link
Author

also, you should separate non-static configuration from static.
static: database connection, redis connection, etc.
non-static: cache enable/disable, other "settings"

@travisdetert
Copy link

There are so many reasons to find a better method to doing this. It also complicates deployment. Dynamically writing out an executable config file? Serious wtf in the choices department here.

@eduard-kistner
Copy link

I am currently having an similar problem with database upgrade on deployment to an ( stage ) environment which runs in production mode.

When i run bin/magento setup:upgrade --keep-generated the system wont do anything than throwing this:

[Exception]
Missing write permissions to the following paths:
/var/www/html/src/app/etc/env.php

Man, i just want ( need ) an DB Upgrade, otherwise Magento wont run throwing an Exception where it asks for an upgrade.
Those details are really frustrating on Magento 2!

@ludwig-gramberg
Copy link
Author

@eduard-kistner yes, it is frustrating, which is why I wrote my own upgrade command which skips the file-checks

@ludwig-gramberg
Copy link
Author

also, see this ticket: #7933

@eduard-kistner
Copy link

@ludwig-gramberg thanks for the information and the corresponding issue.
Will check what they did and if its in 2.1.4 ( maybe you already know ? ).

@NadiyaS
Copy link
Contributor

NadiyaS commented Mar 27, 2017

Hi,
since 2.2.0 we remove the possibility to enable/disable cache from Admin Interface.
Also running CLI commands does not require write permissions for app/etc/env.php, except commands which directly write to this file.

@ludwig-gramberg
Copy link
Author

sounds like 2.2.0 will begin to be a version with developers in mind...

magento 2 has so many large and small quirks which make you want to claw your eyes out to make the pain stop...

related: https://community.magento.com/t5/Version-Upgrades/who-else-is-fed-up-with-blown-up-git-changesets-by-copyright/m-p/70826

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report labels Sep 11, 2017
@geerlingguy
Copy link

geerlingguy commented Nov 13, 2018

Just noting my frustration with this behavior too—I'm trying to deploy Magento 2 in a containerized environment (in Kubernetes), with a static env.php file which is controlled by our VCS and deployment tools, and uses environment variables for many important bits of information.

It seems like, even after Magento is installed, the first page request to any new container will trigger Magento to rewrite the entire contents of the file, which removes any of the environment variables I had in the original file, and then the site starts throwing exceptions. Right now I'm trying to find a way to work around this annoying issue.

If I set the file to read-only, then any time I try loading a page I get:

[Tue Nov 13 17:57:07.365898 2018] [php7:error] [pid 11] [client 172.31.0.1:36806] PHP Fatal error:  Uncaught Magento\\Framework\\Exception\\FileSystemException: The path "/var/www/html/app/etc/env.php" is not writable in /var/www/html/vendor/magento/framework/Filesystem/Directory/Write.php:54\nStack trace:\n#0 /var/www/html/vendor/magento/framework/Filesystem/Directory/Write.php(248): Magento\\Framework\\Filesystem\\Directory\\Write->assertWritable('/var/www/html/a...')\n#1 /var/www/html/vendor/magento/framework/Filesystem/Directory/Write.php(264): Magento\\Framework\\Filesystem\\Directory\\Write->openFile('app/etc/env.php', 'w+')\n#2 /var/www/html/vendor/magento/framework/Code/GeneratedFiles.php(206): Magento\\Framework\\Filesystem\\Directory\\Write->writeFile('app/etc/env.php', '<?php\\nreturn ar...')\n#3 /var/www/html/vendor/magento/framework/Code/GeneratedFiles.php(100): Magento\\Framework\\Code\\GeneratedFiles->enableCacheTypes(Array)\n#4 /var/www/html/vendor/magento/framework/App/ObjectManagerFactory.php(110): Magento\\Framework\\Code\\GeneratedFiles->cleanGeneratedFiles()\n#5 /var/www/html/vendor/magento/framework/App/Bootstrap. in /var/www/html/vendor/magento/framework/Filesystem/Directory/Write.php on line 54

[Edit: Just adding that it seems that I saw some unrelated errors relating to needing to re-run code compilation to populate the generated directory in the replacement/new container—but I still would rather keep this file static/read-only in production if at all possible.]

@theCapypara
Copy link
Member

theCapypara commented Jun 27, 2019

We now also encountered this issue when upgrading to Magento 2.3.2, but with a different message:

In PathValidator.php line 65:

  Path "/src/app/etc/" cannot be used with directory "/src/app/etc/"

Tests confirmed, that it is the same issue (production mode). When env.php is writable, the issue doesn't happen.

This is a big issue for us, because we are using Kubernetes and the env.php is coming from a ConfigMap, which can not be mounted read-write.

@theCapypara
Copy link
Member

The reason for this is the new Observer page_cache_switcher_for_maintenance that tries to disable the PageCache in Magento 2.3.2 when enabling maintenance mode.

@theCapypara
Copy link
Member

This is still an open issue and it just got worse with Magento 2.3.3, because the patch "Magento\Downloadable\Setup\Patch\Data\AddDownloadableHostsConfig" tries to write to the env.php.

Is there any update on this issue?

@engcom-Charlie engcom-Charlie removed the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Dec 23, 2019
@engcom-Charlie engcom-Charlie self-assigned this Dec 23, 2019
@m2-assistant
Copy link

m2-assistant bot commented Dec 23, 2019

Hi @engcom-Charlie. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

    DetailsIf the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

  • 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 4. Verify that the issue is reproducible on 2.4-develop branch

    Details- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 5. Add label Issue: Confirmed once verification is complete.

  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-Charlie engcom-Charlie removed the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Dec 23, 2019
@engcom-Charlie
Copy link
Contributor

engcom-Charlie commented Dec 23, 2019

Hello @ludwig-gramberg

We are not able to reproduce this issue on a fresh Magento 2.4-develop.

Testing scenario:

  1. Set only read permissions for env.php
  2. Run some CLI command

Result:
Commands are working as expected:
image

So i have to close your issue.
Please feel free to comment, reopen or create new ticket according to the Issue reporting guidelines
if you are still facing this issue on the latest 2.4-develop branch. Thank you for collaboration.

Thanks for your report!

@engcom-Charlie engcom-Charlie added Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Progress: needs update labels Dec 23, 2019
@theCapypara
Copy link
Member

@engcom-Charlie This is still an issue, see my comments. This is not the only instance where the file has to be writable.

@ericmorand
Copy link

@engcom-Charlie how about 2.3?

@rain2o
Copy link
Contributor

rain2o commented May 20, 2020

This is still an issue in the most recent production release of Magento - 2.3.5. For us the main issue happens when running bin/magento setup:upgrade --keep-generated after our pipeline runs and we start our docker environment. Our env.php file is also maintained separately and we can't mount it writable. This is causing production to fail every time we deploy.

@ludwig-gramberg
Copy link
Author

almost 4 years and no solution in sight

rant:
why exactly did everyone commit to a platform that was developed from scratch but sold as the successor to a somewhat stable ecommerce platform (that wasn't that great to begin with and didn't scale well?)
magento 1 at least got sh*t down in a fair amount of time, now with magento 2 you can't even get started without getting into the entire SPA universe... imho magento 2 epically failed when they decided to increase complexity by treating a webshop as a web app...

@ericmorand
Copy link

ericmorand commented May 20, 2020

@ludwig-gramberg , I mostly agree and I would add that I don't understand how are people commited to a framework that try to enforce deployment and exploitation workflows to their customers.

@rootindex
Copy link

I am running into this issue on 2.4.7-beta1

If you move the env file to a ConfigMap in k8s this will completely break deployments.

bin/magento set:up --keep-generated

results in

...
Disabling caches:
The "env.php" deployment config file isn't writable.

@hostep
Copy link
Contributor

hostep commented Jul 5, 2023

@rootindex: it's probably better to follow these issues (since these are still in status "Open"):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Progress: needs update Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests