Skip to content
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 incorrect scope condition when populating RHOSTS using services command #18411

Merged

Conversation

rtpt-erikgeiser
Copy link
Contributor

@rtpt-erikgeiser rtpt-erikgeiser commented Sep 29, 2023

When the database contains hosts with empty scope columns (where scope is an empty string), they are not correctly added to RHOSTS via services -R because the command adds a trailing % sign to each address. This can happen if the database is filled by an external program that sets scope to an empty string instead of NULL.

Fixes #18410.

Verification

Make sure you have less then 5 hosts in the database (or add a filter that reduces the number of hits to less than 5) and make sure that the hosts have empty strings in their scope column and not NULL.

  1. msfconsole
  2. services -R

Now you see that the addresses in RHOSTS have trailing % signs. These are normally added to separate the address scope but in this case the scope is empty and they are added regardless.

Fix

The reason why the ternary for adding or omitting the scope to the address fails is because empty strings (like host.scope in this case) are truthy in Ruby. Therefore, the conditional only works if scope is nil. This is fixed by casting potential nil values to a string and performing a more explicit condition (host.scope.to_s != "").

@rtpt-erikgeiser rtpt-erikgeiser force-pushed the fix_incorrect_scope_condition branch from 76b23cf to 4978610 Compare September 29, 2023 11:51
@adfoster-r7
Copy link
Contributor

Thanks for the PR! 👍

Is there replication steps on how to get empty strings for the scope into the database? When running a db_map the scope column is nil and not an empty string, are you running a db_import or similar?

@rtpt-erikgeiser
Copy link
Contributor Author

rtpt-erikgeiser commented Oct 4, 2023

Sorry, I should have added more detailed steps to reproduce the database state.

Here is how you could populate a database, assuming you already have a workspace with ID 1:

insert into hosts (address, workspace_id, scope) values ('127.0.0.1', 1, '');  # the empty string in the scope field is the crucial part
insert into services (port, host_id, proto) values (1234, 1, 'tcp');  # assuming the newly added host has ID 1

Afterwards it can be reproduced as follows:


msf6 > services -R
Services
========

host       port  proto  name  state  info
----       ----  -----  ----  -----  ----
127.0.0.1  1234  tcp

RHOSTS => 127.0.0.1%

I understand that populating the database with 3rd party tools might not be officially supported, but I do think it would be better to handle empty (but not NULL) scopes in a more robust way.

Copy link
Contributor

@smcintyre-r7 smcintyre-r7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a #present? method that can be applied to strings to handle Nil, blank strings and empty strings that would be a better test here.

Even better might be to determine if the scope should be used at all because the address is an IPv6 address. Even if the field is populated for some reason I don't think it should be used for an IPv4 address.

@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Oct 11, 2023

Replication steps

Open up console from msfconsole:

msf6 auxiliary(scanner/postgres/postgres_login) > irb
[*] Starting IRB shell...
[*] You are in auxiliary/scanner/postgres/postgres_login

>> 

Create the database models:

host = Mdm::Host.create!(address: '192.0.2.2', scope: '', workspace: framework.db.workspace)
host.services << Mdm::Service.new(host_id: host, port: 5000, proto: 'tcp')
host.save

Before

Error replicated:


msf6 auxiliary(scanner/postgres/postgres_login) > services -R
Services
========

host       port  proto  name  state  info
----       ----  -----  ----  -----  ----
192.0.2.2  5000  tcp

RHOSTS => 192.0.2.2%

After

RHOSTS no longer has a trailing invalid %

msf6 auxiliary(scanner/postgres/postgres_login) > services -R
Services
========

host       port  proto  name  state  info
----       ----  -----  ----  -----  ----
192.0.2.2  5000  tcp

RHOSTS => 192.0.2.2


@adfoster-r7 adfoster-r7 merged commit 6c33bf9 into rapid7:master Oct 11, 2023
32 checks passed
@adfoster-r7 adfoster-r7 added the rn-fix release notes fix label Oct 11, 2023
@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Oct 11, 2023

Release Notes

Fixes an edge-case where the services -R command generated invalid hosts such as 192.0.2.2% if an empty string was registered for the scope metadata instead of nil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rn-fix release notes fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trailing % signs after addresses when setting RHOSTS via service command
3 participants