In the interest of avoiding side-effects during dumping, I'm
replacing the value with a <constant ast> string instead of
performing an update constant operation.
The only case here that might be *somewhat* sensible is the userdata
argument of array_walk(), which could be used to keep persistent state
between callback invokations -- with the WTF moment that the final
result after the walk finishes will be unchanged. Nowdays, this is
much better achieved using a closure with a use-by-reference.
This causes some tests to fail. Those tests are specifically about
the callback not being able to modify the data though, so this is
clearly not supposed to be a supported use-case.
As part of my work on typed class constants, I wanted to make a
separate pull request for unrelated changes.
These include:
* Moving from ast->child[0]->attr to ast->attr
* Making zend_ast_export_ex() export class constants' visibility
* Extracting an additional zend_compile_class_const_group() function
We modify _basic1.phpt so it runs on Windows as well. The other test
cases hit the issue that `readlink()` fails normally for regular files,
but succeeds on Windows[1]. Therefore, we split these tests, but still
fix the skip reasons.
Most of these have been skipped on Windows for no good reason (`lstat`
is available there as of PHP 4). Several others would only fail,
because the `blksize` and `blocks` elements are always `-1` on Windows,
which can easily be fixed by using `%i` format specifiers instead of
`%d`.
Nikita Popov [Tue, 30 Jun 2020 15:28:47 +0000 (17:28 +0200)]
Remove bogus generator iterator dtor
Fixes a use-after-free encountered in Symfony's SecurityBundle.
I don't have a reproducer for this, and believe the issue can only
occur if we leak an iterator (the leak is a separate issue).
We should not free the generator iterator here, because we do not
own it. The code that fetched the iterator is responsible for
releasing it. In the rare case where we do hit this code-path,
we cause a use-after-free.
Nikita Popov [Tue, 30 Jun 2020 12:40:15 +0000 (14:40 +0200)]
Remove unnecessary ieeefp.h include
This was needed when php_config.h also declare compatibility
shims for isinf() and friends. These are no longer present in
master, so drop this include.
Eddie Kohler [Mon, 8 Jun 2020 12:29:42 +0000 (08:29 -0400)]
Make HashContexts serializable.
* Modify php_hash_ops to contain the algorithm name and
serialize and unserialize methods.
* Implement __serialize and __unserialize magic methods on
HashContext.
Note that serialized HashContexts are not necessarily portable
between PHP versions or from architecture to architecture.
(Most are, though Keccak and slow SHA3s are not.)
An exception is thrown when an unsupported serialization is
attempted.
Because of security concerns, HASH_HMAC contexts are not
currently serializable; attempting to serialize one throws
an exception.
Serialization exposes the state of HashContext memory, so ensure
that memory is zeroed before use by allocating it with a new
php_hash_alloc_context function. Performance impact is
negligible.
Some hash internal states have logical pointers into a buffer,
or sponge, that absorbs input provided in bytes rather than
chunks. The unserialize functions for these hash functions
must validate that the logical pointers are all within bounds,
lest future hash operations cause out-of-bounds memory accesses.
* Adler32, CRC32, FNV, joaat: simple state, no buffer positions
* Gost, MD2, SHA3, Snefru, Tiger, Whirlpool: buffer positions
must be validated
* MD4, MD5, SHA1, SHA2, haval, ripemd: buffer positions encoded
bitwise, forced to within bounds on use; no need to validate
Eddie Kohler [Mon, 22 Jun 2020 03:08:22 +0000 (23:08 -0400)]
SHA-3 Keccak_Hash: Store Keccak_HashInstance in the main context.
Previously, the Keccak_HashInstance was separately allocated.
This could cause memory leaks on errors. For instance,
in php_hash_do_hash_hmac, the following code cleans up after
a file read error:
if (n < 0) {
efree(context);
efree(K);
zend_string_release(digest);
RETURN_FALSE;
}
This does not call the context's hash_final operation, which
was the only way to free the separately-allocated Keccak state.
The simplest fix is simply to place the Keccak_HashInstance state
inside the context object. Then it doesn't need to be freed.
As a result, there is no need to call hash_final in the
HashContext destructor: HashContexts cannot contain internally
allocated resources.