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 reverse_php_ssl infinite loop #9584

Merged
merged 1 commit into from Feb 22, 2018

Conversation

trevorsibanda
Copy link
Contributor

@trevorsibanda trevorsibanda commented Feb 20, 2018

Fix for #8672

Uses php streams to explicitly disable verify_peer option for tls sockets.
A connect timeout of 30 seconds has been added
feof is not suitable for tcp streams, replaced with fgets as while statement predicate

Verification

List the steps needed to make sure this thing works

  • Start msfconsole
  • use use payloads/cmd/unix/reverse_php_ssl
  • set LHOST 0.0.0.0
  • set LPORT 4433
  • generate -t raw
  • ...
  • Setup tls socket listening on port 4433 using self signed certificate
  • Copy generated payload and run. It should connect without flooding with error messages

Additional

@busterb
Copy link
Member

busterb commented Feb 20, 2018

Any compatibility concerns? Looking at https://github.com/rapid7/metasploit-framework/pull/7669/files#diff-33203293cd1e847fee1a744e21c23c92R78 I'm not sure if the later is a little bit cargo cult or which versions of PHP we realistically expect to support here anyway.

@busterb
Copy link
Member

busterb commented Feb 20, 2018

The other thing to do on payload PRs is run ./tools/modules/update_payload_cached_sizes.rb and commit the results.

@egypt
Copy link
Contributor

egypt commented Feb 21, 2018

Looks like stream_context_create was added in PHP 4.3.0, while fsockopen has been around since 4.0. A few years ago, I might have cared about that difference. According to w3techs, PHP4.x is 0.6% of PHP installs on the internet and everything older than PHP4.3 is less than 3% of that.

@egypt
Copy link
Contributor

egypt commented Feb 21, 2018

@busterb The is_callable checks in other PHP payloads are also necessary to determine if the function is in PHP's disabled function list: http://php.net/manual/en/ini.core.php#ini.disable-functions

Copy link
Contributor

@sempervictus sempervictus left a comment

Choose a reason for hiding this comment

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

How far back does this work? Reason for the obscene hackishness of these (breaking env persistence) was to run on what were old php versions back then.
That said, I'm all for the change if it fixes the statelessness.

@trevorsibanda
Copy link
Contributor Author

@sempervictus I've updated the payload cache size . Unless there are other concerns this should be ready

@busterb busterb self-assigned this Feb 22, 2018
@busterb
Copy link
Member

busterb commented Feb 22, 2018

Looks good. I'm happy for the possibility of adding custom ca support with this API too.

@busterb busterb merged commit 77b3673 into rapid7:master Feb 22, 2018
busterb added a commit that referenced this pull request Feb 22, 2018
@busterb
Copy link
Member

busterb commented Feb 22, 2018

Release Notes

This fix prevents the payloads/singles/cmd/unix/reverse_php_ssl module from entering an infinite loop. The payload now uses a more modern PHP API, which improves resiliency and makes it possible to use custom SSL certificate validation in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants