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

PHP Object Injection Security Issue #126

Open
prodigysml opened this issue Feb 3, 2018 · 3 comments
Open

PHP Object Injection Security Issue #126

prodigysml opened this issue Feb 3, 2018 · 3 comments

Comments

@prodigysml
Copy link

@prodigysml prodigysml commented Feb 3, 2018

Expected behavior

When some form data provided by the web application client for unserialize, PHP Object serialization should only be valid for ValidForm objects (or whatever they should be)

Actual behavior

User can enter any PHP Object. This can lead to vulnerabilities like remote code execution and local file inclusion.

* Unserialize previously serialized ValidForm object

This method requires an object to be provided with base64 encoding and gunzip. If we input a serialised PHP object with gunzipped and then base64 encoded, the vulnerability will trigger. I am yet to sift through all the code in the project to find an appropriate class to leverage.

Which version/branch of ValidForm Builder do you use?

I use version/branch: 4.5.4

@flangfeldt

This comment has been minimized.

Copy link
Member

@flangfeldt flangfeldt commented Feb 6, 2018

I'm not sure how this is a problem specific to ValidForm Builder.

The unserialize implementation in this static method is calling the unserialize method of PHP with some decoding and unzipping first. Any object can be unserialized using this method, that is true, but it's not more of a problem than saying that any object can be unserialized by the unserialize method.

Can you give an example of a real world issue?

@prodigysml

This comment has been minimized.

Copy link
Author

@prodigysml prodigysml commented Feb 9, 2018

A possible scenario could be, someone lets a user input serialized ValidForm objects. In such a case, the user can simple input a SimpleXML Object (or any other object they want to leverage) and perform "malicious" actions. I do agree that this is not only your problem, but a simple fix can be ensuring within the unserialize method, that the object provided is actually a ValidForm object. Since the ValidForm object doesn't have any magic methods, successful exploitation is extremely unlikely.

@flangfeldt

This comment has been minimized.

Copy link
Member

@flangfeldt flangfeldt commented Feb 12, 2018

The unserialize method within ValidForm Builder can be seen as a utility method to unserialize any type of objects. It could be a ValidForm object or a ValidWizard object, but also any other kind of object within ValidForm Builder. Maybe someone wants to store individual field objects and add them later to an existing ValidForm object. Testing all possible objects could be done, but I feel it falls outside the scope of the usage intention of the method.

I do agree that a non-static unserialize method should check if the output is actual the object type one would expect. But that is not the case here.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.