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

Security Related Concerns - replace the user of serialize() with json_encode() #265

Open
starshine531 opened this issue Jul 10, 2019 · 2 comments

Comments

@starshine531
Copy link

I'm making a web connector integration with Quickbooks with your SDK and
have a couple of security related concerns.

I noticed that you use unserialize in some places. Somewhat recently,
unserialize was found to be vulnerable to attack. Basically, it's
recommended to never use unserialize on user supplied data. Has your SDK
been updated to address this issue?

Also, I don't see your mysqli driver using parameterized queries. Has
this been updated sufficiently to prevent sql injection?

@starshine531
Copy link
Author

It looks like serialize/unserialize, might be able to be replaced with json_encode/json_decode. From what I have read, json_decode doesn't have the same vulnerability as unserialize because it cannot be decoded into any class, only the stdClass. It also does not use the __wakeup method (which is part of what makes the vulnerability possible). Other than that, it accomplishes the same thing.

@consolibyte
Copy link
Owner

We should be able to replace this with json_encode/decode, and I do think that'd be a good thing. As a temporary work-around, you can make sure you validate any user-supplied input you pass into anything that gets serialized. The $extra parameter to ->enqueue() is just about the only thing serialized here. As a precaution, you should be careful what you pass in there and make sure users aren't passing in classes/objects.

The mysqli driver uses Drupal-style parameter substitution, and is not vulnerable to SQL injections.

@consolibyte consolibyte changed the title Security Related Concerns Security Related Concerns - replace the user of serialize() with json_encode() Jul 15, 2019
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

2 participants