Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Dec 8, 2024

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.1)

Can you help keep this open source service alive? 💖 Please sponsor : )

cmb69 and others added 10 commits December 8, 2024 18:42
These tests execute php.exe without passing the PATH, what might be a
more general issues, but at least with ASan enabled, the required DLLs
could usually not be found, resulting in the tests stalling.

Closes GH-17079.
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.
* PHP-8.3:
  Harden proc_open() against cmd.exe hijacking
* PHP-8.4:
  Harden proc_open() against cmd.exe hijacking
* PHP-8.4:
  Fix GH-17039: PHP 8.4: Incorrect MIME content type
Besides that is generally good practice to avoid macro redefinitions
(and symbol redeclarations), and we're doing this on POSIX platforms
anyway, there is a particular issue regarding phpize builds, where
config.w32.h actually includes config.pickle.h.  The latter overrides
some macro definitions (e.g. `PHP_BUILD_SYSTEM`) to define the proper
values, but if config.w32.h is included multiple times, different macro
definitions eventually raise C4005 compiler warnings[1], which break
builds with `/WX /W1` enabled.

[1] <https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4005>
@pull pull bot added the ⤵️ pull label Dec 8, 2024
@pull pull bot merged commit c0385e9 into wudi:master Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants