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 PHP base64 encoding #17723

Merged
merged 3 commits into from Mar 3, 2023
Merged

Fix PHP base64 encoding #17723

merged 3 commits into from Mar 3, 2023

Conversation

jvoisin
Copy link
Contributor

@jvoisin jvoisin commented Mar 2, 2023

Verification

List the steps needed to make sure this thing works

  • Look at modules/encoders/php/base64.rb
  • See that it doesn't quote the payload
  • Infer that PHP8 will crash with a PHP Fatal error: Uncaught Error: Undefined constant error
  • Despair
  • Verify that the patch does indeed quote the payload now.

@jvoisin jvoisin mentioned this pull request Mar 2, 2023
10 tasks
@gwillcox-r7
Copy link
Contributor

gwillcox-r7 commented Mar 2, 2023

Since the testing directions here don't quite explain the issue let me provide an example:

 ~/git/metasploit-framework │ master ?30  php -v                                                                                                 ✔ │ 9s │ 3.2.1  │ 17:29:04 
PHP 8.1.2-1ubuntu2.11 (cli) (built: Feb 22 2023 22:56:18) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.2, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.2-1ubuntu2.11, Copyright (c), by Zend Technologies
 ~/git/metasploit-framework │ master ?30  cat test.php                                                                                                ✔ │ 3.2.1  │ 17:29:09 
<?php
 eval(base64_decode(ZWNobyAnPHA));
?>
 ~/git/metasploit-framework │ master ?30  php test.php                                                                                                ✔ │ 3.2.1  │ 17:29:13 
PHP Fatal error:  Uncaught Error: Undefined constant "ZWNobyAnPHA" in /home/gwillcox/git/metasploit-framework/test.php:2
Stack trace:
#0 {main}
  thrown in /home/gwillcox/git/metasploit-framework/test.php on line 2
 ~/git/metasploit-framework │ master ?30      

@gwillcox-r7
Copy link
Contributor

gwillcox-r7 commented Mar 2, 2023

And here is confirmation that adding quotes solves this error:

 ~/git/metasploit-framework │ master ?30  cat test.php                                                                                                ✔ │ 3.2.1  │ 17:30:55 
<?php
 eval(base64_decode('ZWNobyAnPHA+SGVsbG8gV29ybGQ8L3A+Jzs='));
?>
 ~/git/metasploit-framework │ master ?30  php -v                                                                                                      ✔ │ 3.2.1  │ 17:30:59 
PHP 8.1.2-1ubuntu2.11 (cli) (built: Feb 22 2023 22:56:18) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.2, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.2-1ubuntu2.11, Copyright (c), by Zend Technologies
 ~/git/metasploit-framework │ master ?30  php test.php                                                                                                ✔ │ 3.2.1  │ 17:31:05 
<p>Hello World</p>                                                                                                                                                                                                                                                                                                               

And with double quotes

 ~/git/metasploit-framework │ master ?30  php test.php                                                                                          ✔ │ 11s │ 3.2.1  │ 17:32:05 
<p>Hello World</p>                                                                                                                                                           ~/git/metasploit-framework │ master ?30  cat test.php                                                                                                ✔ │ 3.2.1  │ 17:32:06 
<?php
 eval(base64_decode("ZWNobyAnPHA+SGVsbG8gV29ybGQ8L3A+Jzs="));
?>
 ~/git/metasploit-framework │ master ?30            

modules/encoders/php/base64.rb Outdated Show resolved Hide resolved
modules/encoders/php/base64.rb Outdated Show resolved Hide resolved
@gwillcox-r7 gwillcox-r7 self-assigned this Mar 2, 2023
@jvoisin
Copy link
Contributor Author

jvoisin commented Mar 3, 2023

What would be the best way to add continuous integration for various php versions to prevent this from happening again?

@gwillcox-r7
Copy link
Contributor

What would be the best way to add continuous integration for various php versions to prevent this from happening again?

@adfoster-r7 Might be the best person to ask r.e this. From our earlier discussion here are a few of the points we talked about. In particular he mentioned that #16357 might be the closest thing we have to running real PHP environments, however that PR isn't ready to land just yet. He also doesn't believe there are any existing patterns for testing encoders but likely to create one it would be a mixture of normal unit tests as well as module tests.

@gwillcox-r7 gwillcox-r7 added bug rn-fix release notes fix labels Mar 3, 2023
@gwillcox-r7 gwillcox-r7 merged commit 6579dcc into rapid7:master Mar 3, 2023
@gwillcox-r7
Copy link
Contributor

Release Notes

A bug has been fixed in the modules/encoders/php/base64.rb encoder whereby strings were being passed as literal strings without being properly quoted, which could result in errors on newer versions of PHP.

@adfoster-r7
Copy link
Contributor

We don't have any existing encoder tests, but if we did we'd most likely add it here:

https://github.com/rapid7/metasploit-framework/tree/6579dcc9773e3a78ab69579c924d1e8701f7c609/spec/modules/auxiliary

Similar to our other module tests which instantiate the module and call the module's public API:

include_context 'Msf::UIDriver'
include_context 'Msf::Simple::Framework#modules loading'
let(:subject) do
load_and_create_module(
module_type: 'auxiliary',
reference_name: 'admin/kerberos/keytab'
)
end

I'm not sure what mocking/glue code would be required to unit test the encoding logic in isolation though - but if you think it would be worthwhile investigating, contributions are welcome - or feel free to ping me if you want any additional guidance on that 👍

@jvoisin jvoisin deleted the fix_php_encoding branch March 5, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug module rn-fix release notes fix
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants