-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Msf::Payload::Adapter::Fetch: Add lwp-request GET fetch adapter #20451
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
base: master
Are you sure you want to change the base?
Conversation
# There is no way to disable cert check in GET ... | ||
print_error('GET binary does not support insecure mode') | ||
fail_with(Msf::Module::Failure::BadConfig, 'FETCH_CHECK_CERT must be true when using GET') | ||
get_file_cmd = "GET -m GET https://#{download_uri}>#{_remote_destination}" |
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.
This is dead code as it will never be called. I copied the same code pattern from the wget
adapter above (which also contains dead code). Presumably the fail_with
was meant to be conditional.
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.
Huh; yes- this came from the certutil command generation. Certutil is a bit of a pain that I don't think you can opt out of the cert check, and this was a way to force the user to accept that limitation, but we don't have that option in here to set. Curious if it was something I forgot to add or something I stripped out after-the-fact.
Regardless, it should not be here, right?
I want to do a check to see if we can set the cert for the httpserver or if certutil simply cannot use https in this case.
#20453
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 copied the same code pattern from the
wget
adapter above (which also contains dead code)
yes- this came from the certutil command generation.
My mistake - you're right, I copied it from the certutil
command generation (not wget
).
Regardless, it should not be here, right?
GET
has the same limitation as certutil
in that insecure certificates cannot be accepted.
This line may be misleading, but leaving it is harmless. The subsequent variable assignment is useless.
I'm fine to leave it in for consistency to be changed when the appropriate functionality is implemented, or change it to a TODO
comment.
print_error('GET binary does not support insecure mode') | ||
fail_with(Msf::Module::Failure::BadConfig, 'FETCH_CHECK_CERT must be true when using GET') | ||
get_file_cmd = "GET -m GET https://#{download_uri}>#{_remote_destination}" | ||
when 'FTP' |
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.
FTP
code path has not been tested.
It looks like this is supposed to be based on fetch_protocol
, but I found no way to control this from within msfconsole
.
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 noticed that we don't have an FTP fetch server recently..... it is on my list of cleanup tasks.
|
def _generate_get_pipe | ||
# Specifying the method (-m GET) is necessary on OSX | ||
execute_cmd = 'sh' | ||
execute_cmd = 'cmd' if windows? |
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 I understand correctly, GET
is only supported by Linux - does this make sense here? Or is it for sake of keeping the pattern?
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 I understand correctly,
GET
is only supported by Linux - does this make sense here? Or is it for sake of keeping the pattern?
It is largely for keeping the pattern. I don't think this conditional will ever be called, as GET
is listed only for Linux options (lib/msf/core/payload/adapter/fetch/linux_options.rb
) but not Windows options (lib/msf/core/payload/adapter/fetch/windows_options.rb
).
GET is supported by Linux and Mac OSX and likely by any other UNIX derived platform which supports Perl and lwp-request. I'm not sure if lwp-request supports Windows (outside of WSL), but it's written in Perl, so maybe. In the event that GET does run on Windows, piping to cmd
(as defined here) should work.
See a previous similar
rex-exploitation
PR here: rapid7/rex-exploitation#22