Reapply "Vulnerability Report Enhancement" #20886
Conversation
This will bring the last fix related to the new Layered Services data model
This reverts commit c355372.
cdelafuente-r7
force-pushed
the
enh/MS-9930/vuln_report_with_creds
branch
from
19eb9d4 to
dabfa48
Compare
| if opts[:sname] | ||
| opts[:name] = opts.delete(:sname) | ||
| end | ||
| opts[:name] = opts[:name].to_s.downcase if opts[:name] |
There was a problem hiding this comment.
Question: It looks like this would be fine moving forwards to have this value downcased, but would there be any issues with the historical data not being downcased? 👀
i.e. if modules previously had report_service(..., sname: 'Foo', ...), we'll end up with a new extra service being added for foo with this new code path? 🤔
A quick eyeball shows that this scenario might not be present, but thought I'd ask 👀
There was a problem hiding this comment.
I think the original code was already down-casing the service name in the loop that assigns the service attributes below. I added this line to make sure it is also down-cased when copying the value to the sopts hash and the DB query will follow the same "downcase" rule.
opts.each { |k,v|
if (service.attribute_names.include?(k.to_s))
service[k] = ((v and k == :name) ? v.to_s.downcase : v)
elsif !v.blank?
dlog("Unknown attribute for Service: #{k}")
end
}
I believe it's fine, but I might be missing something.
| context "with some services" do | ||
| end |
There was a problem hiding this comment.
Yes, absolutely, thanks for the heads up!
| RSpec::Expectations.configuration.max_formatted_output_length = nil | ||
|
|
||
| db.cmd_services | ||
| expect(@output).to match_array [ |
There was a problem hiding this comment.
Not a blocker since this code is written now; but if it helps - I've introduced to match_table that I've found nice to use - it does cleaner diffing and is more copy/paste friendly
Example:
Implementation:
There was a problem hiding this comment.
That's great! I updated it. Thank you!
3 participants
This reapplies the "Vulnerability Report Enhancement" PR. It has been reverted due to issue in Pro, but it came out to be related to something else.
This also bring a fix in
metasploit-credential, see PR. This will need to be landed first before this one.Original PR: #20424
Revert PR: #20777