Use PostgreSQL session type for Postgres modules #18706
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Testing Steps And Outputs Pt. 1
|
Testing Steps And Outputs Pt. 2
|
dwelch-r7
reviewed
Jan 17, 2024
Comment on lines
155
to
160
| vprint_status "#{ip}:#{port} Postgres - querying with '#{sql}'" | ||
| vprint_status "#{rhost}:#{rport} Postgres - querying with '#{sql}'" |
There was a problem hiding this comment.
I think you want to leave these as ip and port no?
you have this above:
ip = opts[:server] || rhost
port = opts[:port] || rport
There was a problem hiding this comment.
After having a look at this, I think this should be rhost and rport. ip and port are defined outside the scope of this function.
| print_good output if datastore['DISPLAY_RESULTS'] | ||
| end | ||
|
|
||
| def get_schema | ||
| ignored_databases = datastore['IGNORED_DATABASES'].split(',').map(&:strip) | ||
| pg_schema = [] | ||
| database_names = smart_query('SELECT datname FROM pg_database').to_a.flatten | ||
| database_names = session ? [session.client.params['database']] : smart_query('SELECT datname FROM pg_database').to_a.flatten |
There was a problem hiding this comment.
does this return the same for both cases? I thought session.client.params['database'] would return the database you're connected to whereas the query gets all the database names in the cluster?
There was a problem hiding this comment.
Correct; but when connecting through a session, only the currently connected DB can be dumped. We cannot switch databases when using a session either, because doing so requires opening a new socket and re-authenticating. This might not work in the case of getting the initial shell through a previous exploit, or when the creds expire etc. This change allows us to dump the database without having to change more code throughout the rest of the module.
Comment on lines
238
to
240
| #vuln_version doesn't seem to work | ||
| #return unless vuln_version? | ||
| return unless vuln_version? |
There was a problem hiding this comment.
did you figure out what the issues were? 👀
sjanusz-r7
force-pushed
the
use-postgresql-session-type-for-postgres-modules
branch
from
0247b50 to
0f9e20c
Compare
sjanusz-r7
force-pushed
the
use-postgresql-session-type-for-postgres-modules
branch
from
0f9e20c to
0cf4d6a
Compare
dwelch-r7
reviewed
Jan 31, 2024
Comment on lines
179
to
+182
| return :not_exists if match_error | ||
|
|
||
| # Default to something sane | ||
| :not_exists |
There was a problem hiding this comment.
Suggested change
| return :not_exists if match_error | |
| # Default to something sane | |
| :not_exists | |
| :not_exists |
I think this can be simplified?
at the minute you're returning :not_exists if match_error is truthy...and then if it's not truthy you end up returning :not_exists anyway
it actually kinda makes the whole match_error bit pointless since it doesn't matter whether it finds the error or not, do you know what makes use of this return value? that might give us a clue as to what was intended here
There was a problem hiding this comment.
I'm willing to change it to something else, maybe :failed or whatever a good alternative would be 👍
The return value is being used in:
def load_extension?(language)
case load_procedural_language(language, 'LANGUAGE')
when ...
when ...
end
Testing
|
sjanusz-r7
force-pushed
the
use-postgresql-session-type-for-postgres-modules
branch
from
29cdb7f to
b2e3e92
Compare
adfoster-r7
reviewed
Feb 9, 2024
| @@ -121,6 +121,14 @@ def initialize(database, user, password=nil, uri = nil) | |||
| end | |||
| end | |||
|
|
|||
| def address | |||
There was a problem hiding this comment.
We'll want to circle back to renaming this to the lexicon similar to peer used elsewhere in metasploit, and other languages
Example ruby snippets:
>> s = TCPSocket.new('127.0.0.1', 5000)
=> #<TCPSocket:fd 18, AF_INET, 127.0.0.1, 52847>
>> s.peeraddr
=> ["AF_INET", 5000, "127.0.0.1", "127.0.0.1"]
>> s.local_address
=> #<Addrinfo: 127.0.0.1:52847 TCP>
Example Python snippets:
>>> import socket
>>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>>> result = s.connect(('127.0.0.1', 5000))
>>> s.getpeername()
('127.0.0.1', 5000)
>>> s.getsockname()
('127.0.0.1', 52883)
adfoster-r7
reviewed
Feb 9, 2024
sjanusz-r7
force-pushed
the
use-postgresql-session-type-for-postgres-modules
branch
from
7f41ff0 to
e71bd70
Compare
sjanusz-r7
force-pushed
the
use-postgresql-session-type-for-postgres-modules
branch
from
e71bd70 to
30fc29e
Compare
sjanusz-r7
commented
Feb 9, 2024
Comment on lines
+41
to
+42
| Msf::Opt::RHOST(nil, false), | ||
| Msf::Opt::RPORT(5432, false) |
There was a problem hiding this comment.
For context, these options were initially added here: 23a9938#diff-5479394bb6dd61d2ac4fc96ae36d29cf5b52e3b7687b7b6c714cbe53b273c4cb
But seemed to be removed here: 2c60780#diff-5479394bb6dd61d2ac4fc96ae36d29cf5b52e3b7687b7b6c714cbe53b273c4cb
Release NotesUpdates multiple PostgreSQL modules to now work with PostgreSQL sessions. This functionality is behind a feature flag which can be enabled with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a continuation of #18663.
This PR changes over Postgres modules so that they are able to target an RHOST/RHOSTS or a PostgreSQL session.
For testing of this PR, I recommend setting up a Metasploitable2 VM which runs Postgres 8.3.1, Docker using
postgres:16orpostgres:16.1(latest as of this PR), and9.2.3(needs a custom Dockerfile setup, described below).Overview
Setup
Docker - Postgres 16/16.x
Docker - Postgres 9.2.3
auxiliary/scanner/postgres/postgres_dbname_flag_injection(all the previous versions are present here):wgetcalls to not use certificates (they're out of date):docker-entrypoint.sh:9.2.3as that's the version we used, but you can uselatestor anything else:Metasploitable2 VM
Verification
Follow the testing steps in the comment below.
msfconsole