security issue: env.php must be writeable in production

[ludwig-gramberg]

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)

[kandy]

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

[ludwig-gramberg]

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]

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

[kandy]

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

[ludwig-gramberg]

it just breaks functionality or security so its bad design

[kandy]

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]

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]

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]

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]

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.