Ocramius
commented
9 years ago Even though I don't see it as a vulnerability (php is full of this sort of
issues with serialized data, regardless if __wakeup or Serializable are
implemented), can we PLEASE keep security issues out of public issue
trackers? Thanks!
On Oct 28, 2014 11:39 AM, "Stanislav Malyshev" notifications@github.com
wrote:
In ZendOauth\Token\AbstractToken, __wakeup looks like:
public function __wakeup() { if ($this->_httpUtility === null) { $this->_httpUtility = new HTTPUtility; } }
This means if I provide _httpUtility in serialized data, it will be kept, whatever it is. Together with toString function, which calls $this->_httpUtility->toEncodedQueryString($this->_params) this provides a way to call function "toEncodedQueryString" on object of any class with any parameters, if the object of AbstractToken class can be substituted in serialized data in place of a string value and thus toString is triggered. Since some classes - such as SoapClient - provide call methods that will accept any method name, this can lead to further execution of code not intended to be executed. This, of course, requires userializing of untrusted data, but this practice is still in use in many apps.
I would recommend removing the if statement and always initialize with new object. Since as per __sleep method _httpUtility is not supposed to be serialized anyway, no loss of functionality would happen because of that.
— Reply to this email directly or view it on GitHub https://github.com/zendframework/ZendOAuth/issues/26.
smalyshev
redacted