Commit edae2431 attempted to fix a leak and double free, but didn't
properly understand what was going on, causing a reference count mistake
and subsequent segfault in this case.
The first mistake of that commit is that the reference count should've
been increased because we're reusing a phar object. The error handling
path should've gotten changed instead to undo this refcount increase
instead of not refcounting at all (root cause of this bug).
The second mistake is that the alias isn't supposed to be transferred or
whatever, that just doesn't make sense. The reason the test
bug69958.phpt originally leaked is because in the non-reuse case we
borrowed the alias and otherwise we own the alias. If we own the alias
the alias information shouldn't get deleted anyway as that would desync
the alias map.
Fixing these will reveal a third issue in which the alias memory is not
always properly in sync with the persistence-ness of the phar, fix this
as well.
Closes GH-17150.
This test puts a fake cmd.exe in the CWD and removes it only after the
test has finished. We need to avoid that other tests are running while
that fake cmd.exe is there, because they may use it instead of the
proper cmd.exe.
We also unlink the fake cmd.exe as soon as possible, regardless of the
test result.
Fixes GH-17098.
Closes GH-17090.
We don't show succeeding tests in the summary, and for all intents and purposes,
these tests have succeeded, in that they behave as expected. I've seen the
output confuse people on multiple occasions, for example GH-17105.
Closes GH-17109
op1 of ZEND_MATCH_ERROR, which refers to the match expression, is not freed by
MATCH_ERROR itself. Instead, it is freed by ZEND_HANDLE_EXCEPTION. For normal
control flow, a FREE is placed at the end of the match expression.
Since FREE may appear after MATCH_ERROR in the opcode sequence, we need to
correctly handle op1 of MATCH_ERROR as alive.
Fixes GH-17106
Closes GH-17108
The directives for FFI should be first in the file, which is fine,
however sometimes there can be comments or whitespace before or between
these defines. One practical example is for license information or when
a user adds newlines "by accident". In these cases, it's quite confusing
that the directives do not work properly.
To solve this, make the zend_ffi_parse_directives() aware of comments.
Closes GH-17082.
Agreed by RM: https://github.com/php/php-src/issues/16168#issuecomment-2525433557
The inline assembly uses labels with the prefix `.L`. On Linux systems
this is the local label prefix. It appears that macOS uses `L` as a
local prefix, which means that the prefix used in the inline assembly is not
local for macOS systems [1].
When combined with inlining, this causes the compiler to get confused
and merge a part of the inline assembly between different functions,
causing control flow to jump from one function to another function.
This is avoided on PHP 8.2 and up by the fact that it
uses `zend_never_inline NOIPA`, but nothing guarantees that compiler
changes won't affect this as well.
To solve this issue, we instead use local labels. These will make the
compiler pick the correct prefix, preventing the issue.
Additionally, while here, we also change the computation of `delta`.
It is undefined behaviour to compute the pointer difference between
two different objects. To circumvent this, we cast first to `uintptr_t`.
This change is cleanly backportable to 8.1 for vendors to pick up.
[1] https://github.com/php/php-src/issues/16168#issuecomment-2404792553
With the help of investigation and testing of @ryandesign.
Closes GH-16348.
Based on the discussion in GH-16286, drop the intl build from macOS + PHP 8.1,
since we cannot build with supported intl versions without too many changes.
Closes GH-17092
See GH-16286
The `jit_prof_threshold` is a float, supposed to be in range [0, 1],
and usually very small (the default is 0.005). Reporting it as int
is meaningless.
Closes GH-17077.
As is, whenever `proc_open()` needs to invoke the shell, cmd.exe is
looked up in the usual executable search path. That implies that any
cmd.exe which is placed in the current working directory (which is not
necessarily what is reported by `getcwd()` for ZTS builds), will be
used. This is a known attack vector, and Microsoft recommends to
always use the fully qualified path to cmd.exe.
To prevent any cmd.exe in the current working directory to be used, but
to still allow users to use a drop in replacement for cmd.exe, we
search only the `PATH` for cmd.exe (and pass the fully qualified path
to `CreateProcessW`), instead of relying on automatic executable search
by passing the base name only.
To be able to easily test this, we provide a minimalist C file which
will be build as test_helper, and used by the new test case.
[1] <https://msrc.microsoft.com/blog/2014/04/ms14-019-fixing-a-binary-hijacking-via-cmd-or-bat-file/>
Closes GH-17043.
Besides that just checking for icuuc.lib does not necessarily imply
that the other libraries are available, doing it this way will not copy
the PDBs to the build folder, so these are not available in the debug
packages. Furthermore, `CHECK_LIB` already adds the library to the
flags, so there is no need to do this manually.
Closes GH-17010.
This test reads an ini "file" from a string, and expects a warning
about locking. But if inifile support is disabled, then you'll get
Warning: dba_open(): Handler "inifile" is not available in
/path/to/ext/dba/tests/gh16390.php on line 3
instead. We skip the test if inifile support is disabled.
Closes GH-17011.
The first while loop sets the bucket variable, and this is freed in
out_failure. However, when the second "goto out_failure" is triggered
then bucket still refers to the bucket from the first while loop,
causing a UAF.
Fix this by separating the error paths.
Closes GH-17058.
There are two functions that can each fail in their own way. If the last
function fails we have to remove the filter entry from the hash table,
otherwise we risk a UAF. Note also that removing the entry from the
table on failure will also free its memory.
Closes GH-17038.
