When the superglobals are eagerly initialized, but "S" is not contained
in `variables_order`, `TRACK_VARS_SERVER` is created as empty array
with refcount > 1. Since this hash table may later be modified, a flag
is set which allows such COW violations for assertions. However, when
`register_argc_argv` is on, the so far uninitialized hash table is
updated with `argv`, what causes the hash table to be initialized, what
drops the allow-COW-violations flag. The following update with `argc`
then triggers a refcount violation assertion.
Since we consider `HT_ALLOW_COW_VIOLATION` a hack, we do not want to
keep the flag during hash table initialization, so we initialize the
hash table right away after creation for this code path.
Closes GH-15930.
We need to avoid signed integer overflows which are undefined behavior.
We catch that, and set `offset` to `ZEND_LONG_MAX` (which is also the
largest value of `zend_off_t` on all platforms). Of course, that seek
may fail, but even if it succeeds, the stream is no longer readable,
but that matches the current behavior for offsets near `ZEND_LONG_MAX`.
Closes GH-15989.
multipart/form-data boundaries larger than the read buffer result in erroneous
parsing, which violates data integrity.
Limit boundary size, as allowed by RFC 1521:
Encapsulation boundaries [...] must be no longer than 70 characters, not
counting the two leading hyphens.
We correctly parse payloads with boundaries of length up to
FILLUNIT-strlen("\r\n--") bytes, so allow this for BC.
We need to avoid signed integer overflows which are undefined behavior.
We catch that, and set `offset` to `ZEND_LONG_MAX` (which is also the
largest value of `zend_off_t` on all platforms). Of course, after such
a seek a stream is no longer readable, but that matches the current
behavior for offsets near `ZEND_LONG_MAX`.
Closes GH-15989.
This was first reported as a leak in GH-15026, but was mistakingly
believed to be a false positive. Then an assertion was added and it got
triggered in GH-15908. This fixes the leak. Upon merging into master the
assertion should be removed as well.
Closes GH-15924.
We're reasonably sure that appending the NUL is not an OOB write, since
the memory stream implementation uses `zend_string` APIs instead of
fiddling with the buffer.
We don't add a regression test because that would require to set up
something in the zend_test extension, and regressions are supposed
to be caught by external consumers of this API, such as mailparse.
Closes GH-15648.
Although the issue was demonstrated using Curl, the issue is purely in
the streams layer of PHP.
Full analysis is written in GH-11078 [1], but here is the brief version:
Here's what actually happens:
1) We're creating a FILE handle from a stream using the casting mechanism.
This will create a cookie-based FILE handle using funopen.
2) We're reading stream data using fread from the userspace stream. This will
temporarily set a buffer into a field _bf.base [2]. This buffer is now equal
to the upload buffer that Curl allocated and note that that buffer is owned
by Curl.
3) The fatal error occurs and we bail out from the fread function, notice how
the reset code is never executed and so the buffer will still point to
Curl's upload buffer instead of FILE's own buffer [3].
4) The resources are destroyed, this includes our opened stream and because the
FILE handle is cached, it gets destroyed as well.
In fact, the stream code calls through fclose on purpose in this case.
5) The fclose code frees the _bs.base buffer [4].
However, this is not the buffer that FILE owns but the one that Curl owns
because it isn't reset properly due to the bailout!
6) The objects are getting destroyed, and so the curl free logic is invoked.
When Curl tries to gracefully clean up, it tries to free the buffer.
But that buffer is actually already freed mistakingly by the C library!
This also explains why we can't reproduce it on Linux: this bizarre buffer
swapping only happens on macOS and BSD, not on Linux.
To solve this, we switch to an unbuffered mode for cookie-based FILEs.
This avoids any stateful problems related to buffers especially when the
bailout mechanism triggers. As streams have their own buffering
mechanism, I don't expect this to impact performance.
[1] https://github.com/php/php-src/issues/11078#issuecomment-2155616843
[2] 5e566be7a7/stdio/FreeBSD/fread.c (L102-L103)
[3] 5e566be7a7/stdio/FreeBSD/fread.c (L117)
[4] 5e566be7a7/stdio/FreeBSD/fclose.c (L66-L67)
Closes GH-14524.
