Skip to content

[Security] Add caution on symfony cli web server exposing env vars on private network #17309

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

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

94noni
Copy link
Contributor

@94noni 94noni commented Sep 30, 2022

Hi,

One can understood that this web server is a great tool for development purposes but this addition should be added imho for knowledge :)

Context:

when checking some local data accessible on local network with coworker
we arrived to display a symfony cli served app profiler (obviously it is in dev env)
and in the profiler > request/response panel > server parameters > regular env vars => thus exposing also symfony unrelated env vars which are included

friendly ping @wuchen90 ^^

@javiereguiluz
Copy link
Member

I'm divided about adding this. I thought it was clear that this CLI + local server is only for the dev environment ... so it should never be used in production or any other real servers.

But, let's wait to read more opinions. Thanks.

@94noni
Copy link
Contributor Author

94noni commented Oct 4, 2022

@javiereguiluz

so it should never be used in production

we are not using it in prod at all, but in development locally but as it run a web server, one can find it on local private network and see unrelated env vars

@wouterj wouterj modified the milestones: 6.2, 4.4 Oct 18, 2022
@javiereguiluz javiereguiluz added Waiting team decision Request for comments from Symfony Docs Team members and removed Waiting feedback labels Oct 26, 2022
@javiereguiluz
Copy link
Member

@nicolas-grekas what do you think about this proposal? Thanks

@nicolas-grekas
Copy link
Member

This looks unrelated to the symfony binary. Any other webserver will do the same, isn't it?

@94noni
Copy link
Contributor Author

94noni commented Jan 17, 2023

This looks unrelated to the symfony binary

perhaps indeed; but to me it is worth noticing in the doc, thus this PR :)
obviously feel free to close it if you think it is not worth the "security issue" it can rise

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it doesn't hurt to add this, but should we target 5.4?

@xabbuh xabbuh modified the milestones: 4.4, 5.4 Jan 20, 2023
@94noni 94noni changed the base branch from 6.2 to 5.4 February 25, 2023 21:53
@94noni 94noni changed the base branch from 5.4 to 6.2 February 25, 2023 21:53
@javiereguiluz javiereguiluz added Status: Reviewed and removed Waiting team decision Request for comments from Symfony Docs Team members Status: Needs Review labels Apr 4, 2023
@javiereguiluz javiereguiluz merged commit 864c028 into symfony:6.2 Apr 4, 2023
@javiereguiluz
Copy link
Member

I merged this but I made some changes:

  • Mention that this is done by most web servers, not just this web server
  • Move this caution at the bottom of the article where env vars are mentioned din the Docker section (otherwise, you find this intimidating caution note at the beginning of the article, which could cause a bad impression on some readers)

See e81818e

Thanks!

@94noni
Copy link
Contributor Author

94noni commented Apr 4, 2023

@javiereguiluz this make sens indeed, thx for taking care of this PR
have a good day!

@94noni 94noni deleted the patch-10 branch April 4, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants