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 bad loop terminator checks and data checks in memcached_extractor.rb #16704

Merged
merged 2 commits into from
Jul 1, 2022

Conversation

gwillcox-r7
Copy link
Contributor

Fixes #15975 by adding in additional error handling and checks to break out of the loop

Verification

List the steps needed to make sure this thing works

  • Start msfconsole
  • use auxiliary/gather/memcached_extractor
  • Contact me for the target host.
  • set RHOST <IP of target>
  • run
  • Verify that the program no longer errors out with the type conversion error.
  • Test this on any other hosts n verify it still works correctly and that my additional code changes look correct.

…g and recieving only the first 4096 bytes of data and then executing the query again
@@ -54,12 +54,13 @@ def localhost?(ip)
def enumerate_keys
keys = []
enumerate_slab_ids.each do |sid|
sock.send("stats cachedump #{sid} #{max_keys}\r\n", 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does max_keys here end up a reason for iteration or is the response guaranteed to be all keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So according to https://elijaa.org/2010/12/24/understanding-memcached-stats-cachedump-command.html this is simply the maximum number of keys to dump however supposedly there may be a maximum return size of 1 MB, though given this command isn't documented and is part of the undocumented interface, there is speculation it may be 2 MB but not confirmed.

This is also noted at https://lzone.de/blog/How-to-Dump-Keys-from-Memcache where they note the following constraints:

  1. You can only dump keys per slab class (keys with roughly the same content size)
  2. You can only dump one page per slab class (1MB of data)
  3. This is an unofficial feature that might be removed anytime.

Hopefully that helps explain things a bit here.

@cdelafuente-r7 cdelafuente-r7 self-assigned this Jun 28, 2022
@cdelafuente-r7
Copy link
Contributor

Thanks @gwillcox-r7 ! I verified it fixed the issue on faulty targets and also still work on normal ones. I'll go ahead and land it.

Before the fix

msf6 auxiliary(gather/memcached_extractor) > run verbose=true RHOSTS=<redacted>

[*] 123.207.184.223:11211 - Connecting to memcached server...
[+] 123.207.184.223:11211 - Connected to memcached version 2.5.1
[*] 123.207.184.223:11211 - Using lru_crawler to enumerate keys
[*] 123.207.184.223:11211 - Error: <redacted>: TypeError no implicit conversion of nil into Array
[*] 123.207.184.223:11211 - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed

With the fix

msf6 auxiliary(gather/memcached_extractor) > run verbose=true RHOSTS=<redacted>

[*] 123.207.184.223:11211 - Connecting to memcached server...
[+] 123.207.184.223:11211 - Connected to memcached version 2.5.1
[*] 123.207.184.223:11211 - Using lru_crawler to enumerate keys
[+] 123.207.184.223:11211 - Found 0 keys
[*] 123.207.184.223:11211 - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed

@cdelafuente-r7 cdelafuente-r7 added rn-fix release notes fix bug labels Jul 1, 2022
@cdelafuente-r7 cdelafuente-r7 merged commit 9de7411 into rapid7:master Jul 1, 2022
@cdelafuente-r7
Copy link
Contributor

Release Notes

This fixes an issue when targeting some faulty memcached servers that return an error when extracting the keys and values stored in slabs. The module no longer errors out with a type conversion error.

@gwillcox-r7 gwillcox-r7 deleted the fix-memcached-extractor branch July 1, 2022 14:57
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug rn-fix release notes fix
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

auxiliary/gather/memcached_extractor type error
4 participants