When a uint8_t is bitshifted to the left, it is actually promoted to an
int. For the current code this has the effect of a wrong sign-extension,
and the result will also wrongly become zero when insert_pos >= 32.
Fix this by adding an explicit cast.
Furthermore, the partial prefix byte mask was computed incorrectly: the
byte is already shifted so the mask should not account for the shift.
The CSPRNG failing should be rare nowadays, but it *might* happen and without
this patch it's hard for the user to find out why the salt generation failed:
The error message is not actionable.
This patch will automatically set the CSPRNG exception to the `$previous`
exception of the ValueError that is thrown, allowing the developer to determine
the cause of the salt generation failure.
Before:
Fatal error: Uncaught ValueError: Unable to generate salt in php-src/test3.php:3
Stack trace:
#0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y')
#1 {main}
thrown in php-src/test3.php on line 3
After:
Fatal error: Uncaught Random\RandomException: Cannot open /dev/urandom: No such file or directory in php-src/test3.php:3
Stack trace:
#0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y')
#1 {main}
Next ValueError: Unable to generate salt in php-src/test3.php:3
Stack trace:
#0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y')
#1 {main}
thrown in php-src/test3.php on line 3
The only way the previous `if (read_bytes < size)` branch could be taken is
when the loop was exited by the `break;` statement. We can just merge this into
the loop to make the code more obvious.
This effectively reverts #8984.
As discussed in #10327 which will enable the use of the getrandom(2) syscall on
NetBSD instead of relying on the userland arc4random_buf(), the CSPRNG should
prioritize security over speed [1] and history has shown that userland
implementations unavoidably fall short on the security side. In fact the glibc
implementation is a thin wrapper around the syscall due to security concerns
and thus does not provide any benefit over just calling getrandom(2) ourselves.
Even without any performance optimizations the CSPRNG should be plenty fast for
the vast majority of applications, because they often only need a few bytes of
randomness to generate a session ID. If speed is desired, the OO API offers
faster, but non-cryptographically secure engines.
It cannot be decided whether the device is available at build time, PHP might
run in a container or chroot that does not expose the device. Simply attempt to
open it, if it does not exist it will fail.
This improves readability of php_random_bytes() by removing one layer of
preprocessor conditions.
This moves them from ``.data`` to ``.rodata`` and allows more compiler optimizations.
* ext/opcache/zend_accelerator_hash: make prime_numbers const
* Zend/zend_signal: make zend_sigs const
* ext/dba: make dba_handler pointers const
* ext/exif: make php_tiff_bytes_per_format and other globals const
* ext/intl/grapheme: make grapheme_extract_iters const
* ext/mstring: make rare_codepoint_bitvec const
* ext/snmp: make objid_mib const
* ext/opcache: make all zend_shared_memory_handlers const
This was introduced in 3467526a65 and the
corresponding RFC gives some reasoning. However the CSPRNG being “not secure
enough” is not a thing and reading these extra bytes is just security theater:
If the CSPRNG would hypothetically be broken, then PHP’s session IDs are the
least of one’s concerns, because we already trust it in `random_bytes()` and
might generate long-term secrets using that.
Alex Dowad noticed[1] that the SIMD stripslashes implementation actually
only depended on SSE2, and not on SSE4.2 instructions. Remove the
checking for SSE4.2 and only check for SSE2. This also greatly
simplifies the supporting code.
[1] https://github.com/php/php-src/pull/10313#issuecomment-1382825073
Allow to bind a socket to a divert port without being concerned
by its address. for ipfw filter purpose (SO_USER_COOKIE constant).
FreeBSD only.
Close GH-10415.
As an example:
should be translated to:
ZVAL_LONG(&attribute_Attribute_class_test_arg0, ZEND_ATTRIBUTE_TARGET_FUNCTION | ZEND_ATTRIBUTE_TARGET_METHOD);
remove always false condition from exif
The latter condition will never trigger because otherwise the do-while loop
wouldn't have exited.
Close GH-10402
The assertion failure was triggered in a debug code-path that validates
property types for internal classes.
zend_verify_internal_read_property_type was called with retval being a
reference, which is not allowed because that function eventually calls to
i_zend_check_property_type, which does not expect a reference.
The non-debug code-path already takes into account that retval can be a
reference, as it optionally dereferences retval.
Add a dereference in zend_verify_internal_read_property_type just before
the call to zend_verify_property_type, which is how other callers often
behave as well.
I learned this trick for doing a faster bounds check with both upper
and lower bounds by reading a disassembler listing of optimized code
produced by GCC; instead of doing 2 compares to check the upper and the
lower bound, add an immediate value to shift the range you are testing
for to the far low or high end of the range of possible values for the
type in question, and then a single compare will do. Intstead of
compare + compare + AND, you just do ADD + compare.
From microbenchmarking on my development PC, this makes strtoupper()
about 10% faster on long strings (~10,000 bytes).
Previously function JIT for the whole script (opcache.jit=1205) dumped
all op_arrays before JIT-ing the first functon. This complicated debugging
of particular JIT-ed function and leaded to usage of opcache.jit=1204.
Now both options should produce similar op_array/disasm interlived
output.
When this INI option is enabled, it reverts the line separator for
headers and message to LF which was a non conformant behavior in PHP 7.
It is done because some non conformant MTAs fail to parse CRLF line
separator for headers and body.
This is used for mail and mb_send_mail functions.
* Revert "ext/opcache: use C11 atomics for "restart_in" (#10276)"
This reverts commit 061fcdb0a5.
While the commit does indeed improve performance, @dstogov complained
that it disables the code path calling kill_all_lockers(), and thus
hanging workers are never killed and restarted.
https://github.com/php/php-src/pull/10276#issuecomment-1383593046
The fact that this feature was not implemented in the existing atomic
code path (i.e. Windows) did not mean that the feature was considered
not strictly necessary, but that the Windows implementation just did
not need the feature because SAPIs that work on Windows do not manage
child processes
https://github.com/php/php-src/pull/10276#issuecomment-1383868696https://github.com/php/php-src/pull/10276#issuecomment-1384235011
* ext/opcache: document lack of kill_all_lockers() on Windows
kill_all_lockers() is not implemented on Windows, and does not need to
be.
Upon freeing libxslt's context, every document which is not the *main*
document will be freed by libxslt. If a node of a document which is not
the main document gets returned to userland, we'd free the node twice:
- first by the cleanup of the xslt context
- and then by our own refcounting mechanism.
This was reported in bug 49634, and was fixed by always copying the
node (and later re-fixed in bug 70078).
The original fix is not entirely correct unfortunately because of the
following two main reasons:
- modifications to the node will only modify the copy, and not the original
- accesses to the parent, path, ... will not work
This patch fixes it properly by only copying the node if it origins from
a document other than the main document.
Co-authored-by: juha.ikavalko@agentit.fi
Closes GH-10318.
