recipes icon indicating copy to clipboard operation
recipes copied to clipboard

The default security configuration requires a trailing forward slash when accessing the profiler

Open aarong416 opened this issue 2 years ago • 3 comments

I created a new Symfony 5.4 project (5.4 specifically because I needed to use PHP 7.4), using symfony new --webapp, and the security configuration (config/packages/security.yml) generated by default has the following firewall:

dev:
    pattern: ^/(_(profiler|wdt)|css|images|js)/
    security: false

The problem with the pattern is that it doesn't match the URL localhost:8000/_profiler, but it does match the URL localhost:8000/_profiler/ (note the trailing `/). This means that when accessing the profiler, a trailing forward slash has to be added in at the end, which might not be added by the browser, otherwise you get redirected to the login screen.

But, if you navigate to localhost:8000/_profiler/ (with a trailing /), the dev firewall is used and the profiler can be accessed without authentication.

The way I came upon this problem is because I have a login screen that isn't working and so I needed to debug it. I navigated to localhost:8000/_profiler, but it redirected me to the login page, but since the login page wasn't working, I wasn't sure if it was possible to access the profiler, even though I know it should be accessible from an unauthenticated context.

So the problems here are:

1.The default security configuration leads you to think that you have to be authenticated to be able to access the profile, when it can be accessed by an unauthenticated user. 2. The default security configuration forces developers to add the trailing / in the URL when accessing the profiler.

The fix for this was to add a ? at the end of the regular expression to make the trailing / optional:

pattern: ^/(_(profiler|wdt)|css|images|js)/?

I suggest that this is the change that needs to be made, so that it doesn't need to be added by developers whenever they create new projects.

I had a look at this repo for a CONTRIBUTING.md file, but there is none, so I'm not sure if there's a way I can contribute other tha creating an issue. If there, let me know and I'd be happy to make changes and create a PR.

aarong416 avatar May 18 '23 11:05 aarong416

I totally agree with your assessment! However, we might need to "live with this". The problem with making the / at the end optional is that this would now, potentially catch other paths - e.g. /profilersection. It's a total edge case, but it's possible.

If we WERE to improve this, it would need to match EXACTLY /profiler or /profiler/*, but not /profiler*.

weaverryan avatar May 18 '23 15:05 weaverryan

This could be done by changing the end of the regex, replace / with (/|$)

stof avatar Jun 05 '23 16:06 stof