-
Notifications
You must be signed in to change notification settings - Fork 14k
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
Fix query param in reconfig_redirect_opts! #19700
Fix query param in reconfig_redirect_opts! #19700
Conversation
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.
I tested it with the module multi/http/wso2_api_manager_file_upload_rce
and it is working just fine.
if location.query | ||
opts['query'] = location.query | ||
else | ||
opts['query'] = '' | ||
end |
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.
if location.query | |
opts['query'] = location.query | |
else | |
opts['query'] = '' | |
end | |
opts['query'] = location.query |
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.
Looks like we default to ''
in the bowels of rex, not a blocker
'query' => '', |
BeforePrevious params incorrectly used in redirect
AfterNo query params added
|
Release NotesFixes a bug where HTTP redirects were not handling HTTP query parameters correctly |
The
reconfig_redirect_opts!
method inhttp_client.rb
seems to be incorrectly setting theopts['query']
parameter when usingsend_request_cgi!
to follow redirects.If your original request is:
and the response to that request has a location header of:
The URI of the next request should be the location header mentioned above however because
opts['query']
(the GET request parameter(s) from the original request) is already set, it doesn't get reset, so the next request ends up being:Which seems to be an incorrect combination of the original requests's parameters with the location header's path.
I noticed this when I was attempting to rewrite the manual handling of redirection here:
https://github.com/rapid7/metasploit-framework/pull/19647/files#diff-0c05ed862598aef0fcdd978f22c5676c187ea3d7d41da4de51627170fed6f455R195-R211
Below are some HTTP trace logs from the failed attempt of trying to replace the manual redirection with
send_request_cgi!
.HTTP trace of current incorrect behaviour
After implementing this change the module works correctly when using
send_request_cgi!
instead of manually handing the redirection.