-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Vulnerability fix: validate return_to param using request.host #3627
Vulnerability fix: validate return_to param using request.host #3627
Conversation
@@ -56,7 +56,7 @@ def respond_to_missing?(sym, include_private) | |||
end | |||
|
|||
def back_or_index | |||
params[:return_to].presence && params[:return_to].include?(request.host) && (params[:return_to] != request.fullpath) ? params[:return_to] : index_path | |||
params[:return_to].presence && params[:return_to].starts_with?(request.host) && (params[:return_to] != request.fullpath) ? params[:return_to] : index_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But request.host
is in the form of www.example.com
. So this won't allow the valid url https://www.example.com/
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, we need to take the scheme into account. I'll fix it. Thx 🙌 !
72d1ab8
to
287e207
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready for another round.
The failing check is rubocop in:
Is this being addressed? Let me know how should I proceed |
Merged in. Thank you for the fix! |
The Rubocop failure was already addressed in the master: |
* 'master' of github.com:railsadminteam/rails_admin: Tidy up gemfiles Bump jRuby Test against Mongoid 8 Test against Rails 7.1 Mitigate jRuby build failures Upgrade node and vite to fix the vite build Un-pin turbo-rails to fix Rails 6.1 builds Follow-up for railsadminteam#3555 Resolved a problem with embedded mongoid documents and boolean selectors (railsadminteam#3555) Stop using update_only to decide to show subform on create (railsadminteam#3649) Follow-up for railsadminteam#3643 Basic vite integration attempt (railsadminteam#3643) Support client-side dynamic scoping Fix Rubocop offense Revive the live demo Unlock webrick version Fix typo: tripple ==> triple (railsadminteam#3637) Replace ” entity with inline <q> element (railsadminteam#3636) Vulnerability fix: validate return_to param using request.host (railsadminteam#3627) Fix Rubocop offense
Context
An internal team audits the Maxwell Rails apps occasionally, and in the last run, they discovered a vulnerability. Please take a look at the details bellow:
Solution
Validate the URL starts with the
request.host
request.base_url