String matching and handling fixes in aws_ec2_instance_metadata module #10338
Conversation
| base_resp.split(/\r\n/).each do |l| | ||
| new_uri = base_uri.merge("./#{l}") | ||
| if l =~ %r{/$} | ||
| base_resp.split(/\n/).each do |l| |
There was a problem hiding this comment.
Is there a reason that .split(/\r\n/) was replaced with .split(/\n/) ? This is kind of important.
If it was causing problems, consider using /\r?\n/ instead.
| @@ -36,7 +36,7 @@ def initialize(info = {}) | |||
|
|
|||
| def check_aws_metadata | |||
| resp = simple_get(@target_uri) | |||
| unless resp =~ /^instance-id.$/m | |||
| unless resp =~ /instance-id/m | |||
There was a problem hiding this comment.
Without context, I'm not sure why this regex needed to be replaced. However, if the change makes sense, then regex is unnecessary here and resp.to_s.include?('instance-id') is preferred.
| if l =~ %r{/$} | ||
| base_resp.split(/\n/).each do |l| | ||
| new_uri = "#{base_uri}#{l}" | ||
| if l =~/\/$/ |
There was a problem hiding this comment.
if l =~ %r{/$} and if l =~/\/$/ are functionally identical.
Not sure why this was changed? %r{} notation is preferred over escaping /.
However, if l is only ever going to be one line, then l.end_with?('/') could be used instead, and is preferred.
| r[l.gsub(%r{/$}, '')] = get_aws_metadata(new_uri, simple_get(new_uri)) | ||
| elsif new_uri.to_s =~ %r{/public-keys/} && /^(?<key_id>\d+)=/ =~ l | ||
| r[l.gsub(/\/$/, '')] = get_aws_metadata(new_uri, simple_get(new_uri)) | ||
| elsif new_uri.to_s =~ /\/public-keys/ && /^(?<key_id>\d+)=/ =~ l |
There was a problem hiding this comment.
Same here. %r{} notation is preferred over escaping.
|
It is required that code in your fork be merged from a unique branch in your repository to master in Rapid7's. Please create a new branch in your fork of framework and resubmit this from that branch. This helps protect the process, ensure users are aware of commits on the branch being considered for merge, allows for a location for more commits to be offered without mingling with other contributor changes and allows contributors to make progress while a PR is still being reviewed. Closing based on the this requirement, please do resubmit from a unique branch. |
|
@bcoles I am not an expert in Ruby so can not properly justify my choices. I couldn't run the script due to different errors (like merging issues), however the modified version I was able to run and I received the expected output. |
Fixes for Kali linux 4.14 with ruby 2.3.
Tell us what this change does. If you're fixing a bug, please mention
the github issue number.
Verification
List the steps needed to make sure this thing works
msfconsoleuse exploit/windows/smb/ms08_067_netapi