Set Cross-Origin-Opener-Policy: same-origin on all pages #5442
Conversation
|
cc @segiddins @duckinator since you are working on the report. |
|
cc @segiddins @duckinator gentle ping on this one. |
|
@alikabeel-spooner people will check once they will find some time, no need to keep pinging |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5442 +/- ##
==========================================
- Coverage 97.06% 6.54% -90.52%
==========================================
Files 451 452 +1
Lines 9391 13171 +3780
==========================================
- Hits 9115 862 -8253
- Misses 276 12309 +12033 ☔ View full report in Codecov by Sentry. |
| @@ -84,10 +88,6 @@ class Application < Rails::Application | |||
| config.active_support.cache_format_version = 7.1 | |||
|
|
|||
| config.action_dispatch.rescue_responses["Rack::Multipart::EmptyContentError"] = :bad_request | |||
|
|
|||
| config.action_dispatch.default_headers.merge!( | |||
There was a problem hiding this comment.
should we instead upstream this to rails so the default headers also apply on error pages?
There was a problem hiding this comment.
I am not an expert in rails. So, if you think it would work, we can surely try it. I was just trying to create something that works and fixes the issue but need your kind help to get this done.
There was a problem hiding this comment.
I was about to ask the same. 💪 This seems good default for new apps.
There was a problem hiding this comment.
Totally good to me! If you can do that or guide me on what to do, it would be great.
There was a problem hiding this comment.
I found out Rails as a bug bounty program where I can report this issue to see if they are interested in fixing. Since this can take time, I am not sure if in the meanwhile we want to merge this temporary patch?
There was a problem hiding this comment.
I'm affraid this patch is not worth any bug bounty program.
There was a problem hiding this comment.
@simi actually the COOP header protects against a class of XS-Leaks using Cross Window Navigation. It is usually accepted as a low/medium issue in BBPs.
There was a problem hiding this comment.
or maybe https://github.com/sinatra/sinatra/tree/main/rack-protection could be good target to contribute
There was a problem hiding this comment.
I will look into that as well. Thanks! How do you prefer to tackle this PR until i figure out whether it is rails or sinatra?
Hello Team,
This PR fixes HackerOne Report 2681420. In the previous fix #4910, @segiddins add the header to all standard pages (not errors). However, the fix did not cover the cases of non-existing paths (errors). This PR fixes the issue by ensuring all pages, even errors, have the header correctly set. I tested this locally, and it should work fine on all paths. Can you kindly review and merge this PR and then close the H1 report so that I can claim the bounty?
Best,
Ali