PHP Object Injection vulnerability due to misuse of unserialize allowed_classes

[mcdruid]

I submitted this privately to Google's OSS VRP but they decided it doesn't meet the requirements to be handled as a security issue.

The AMP Toolbox for PHP has a potential PHP Object Injection vulnerability that could be exploited by an attacker that can write files to the system temp directory. (Acknowledging that this is a "high barrier" for the problem to be exploitable - it would require something like an upload form that writes files directly to the temp directory with the attacker's chosen filename.)

A successful exploit could escalate the ability to write to the temp directory. Depending on what Gadget Chains are available in the application, full Remote Code Execution could be achieved.

The vulnerability is caused by an improperly formatted parameter to unserialize().

                $cachedResponse = unserialize($cachedResponse, [RemoteGetRequestResponse::class]);

This passes a second parameter to unserialize() which is presumably intended to limit the allowed classes that can be unserialized. However, the format of this parameter is incorrect:

https://www.php.net/manual/en/function.unserialize.php

The 2nd parameter is an array of options, and there should be an element with the key allowed_classes. The function call should be:

unserialize($cachedResponse, ['allowed_classes' => [RemoteGetRequestResponse::class]]);

The current implementation does not restrict the classes that can be unserialized and therefore a malicious payload in an appropriately named file in the temp directory could be used to achieve PHP Object Injection.

[mcdruid]

Running this test without fixing the issue results in an expected failure:

There was 1 failure:

1) AmpProject\RemoteRequest\TemporaryFileCachedRemoteGetRequestTest::testPHPObjectInjection
Failed asserting that file "/path/to/amp-toolbox-php/pi.php" does not exist.

/path/to/amp-toolbox-php/tests/RemoteRequest/TemporaryFileCachedRemoteGetRequestTest.php:162

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ swissspidy
❌ mcdruid
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[swissspidy]

@mcdruid Thank you for the contribution! Could you please sign the CLA so that we can merge the PR?