Hi internals,

I plan to start the vote for 
https://wiki.php.net/rfc/phar_stop_autoloading_metadata on 2020-07-21, in 6 
days.

I've created an implementation of this RFC that passes existing phar test cases 
( https://github.com/php/php-src/pull/5855 ).
If anyone has additional test cases, patches, fixes, or improvements (or bug 
reports for this PR), I'd love to see them.

This RFC proposes to not unserialize the metadata automatically when a phar is 
opened by php (Previously, it did). It will make PHP unserialize the metadata 
**only** if Phar->getMetadata() or PharFile->getMetadata() is called directly. 
(as described in https://bugs.php.net/bug.php?id=76774)

- I plan to add more `ZEND_ASSERT` assertions that persistent phars added in 
`phar.cache_list` don't have temporary zvals (e.g. objects) created in a place 
where permanent zvals were expected. (probably by avoiding storing any zvals)
  (and/or stop storing the results of unserialize())
- I plan to look into early returns if serialize()/unserialize() calls throw a 
Throwable. This should not affect stream wrappers, only explicit uses of 
metadata from Phar or PharFile objects.
- If any unexpected issues do get introduced here, I'd anticipate they'd be 
limited to explicit calls from PHP to Phar or PharFile's 
setMetadata/getMetadata/delMetadata,
  which should have less security impact than prior to this RFC, where 
`file_exists("phar://$untrusted")` can lead to a call to unserialize().  (see 
the RFC for security concerns of phar stream wrappers )
- I'd expect that any unanticipated issues could be solved by the first release 
candidate is released

> I've created https://wiki.php.net/rfc/phar_stop_autoloading_metadata as 
> mentioned earlier in https://externals.io/message/110856
> 
> This aims to add the mitigations described in 
> https://externals.io/message/105271#105291 , which seemed to be the most 
> straightforward approach to avoiding unexpected side effects of 
> unserialization.
> - For a trusted phar, I wouldn't expect to need to unserialize metadata to 
> check for the file not being corrupt (e.g. there's a checksum, and people 
> would have tested the phar manually).
> - For an untrusted phar, I'd want php to avoid calling unserialize() when 
> reading it.
> 
> https://bugs.php.net/bug.php?id=76774 goes into more detail about the 
> security issues this aims to fix. 

Thanks,
- Tyson

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to