Skip to content

appsec: mitigate PHP2Shell on unpatched PHP (CVE-2026-109618/109619) - #4383

Draft
cataphract wants to merge 3 commits into
masterfrom
glopes/appsec-oom-soft-limit
Draft

cataphract wants to merge 3 commits into
masterfrom
glopes/appsec-oom-soft-limit

Conversation

@cataphract

@cataphract cataphract commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Description

Mitigates the PHP2Shell vulnerabilities (CVE-2026-109618 and CVE-2026-109619) in the appsec extension, for customers running PHP releases older than the upstream fix (8.2.35 / 8.3.36 / 8.4.27 / 8.5.12).

CVE-2026-109618. On unpatched PHP, a memory_limit error raised while a key is being inserted into a hash table (_zend_hash_str_add_or_update_i) bails out mid-insertion, and request shutdown then walks the half-initialised bucket. Request variables are parsed before any userland runs, so the request alone can reach it; parse_str() and other char *-key builders reach it at runtime.

While the heap is armed, our Zend MM handlers serve every allocation and the engine limit is lifted to a sentinel, so no insertion is abandoned: crossing the soft limit (memory_limit) makes the error pending instead.

  • Parse windows. The startup treat_data parse (before RINIT) and request_parse_body() (8.4+) get a window of their own. Once the error is pending, an input_filter wrapper drops later variables, an rfc1867 FILE_START wrapper skips later file parts and a read_post wrapper ends a multipart body; the error is raised as a normal fatal once the arrays are complete. Hard limit: min(max(L, S + k·B + 8 MiB), S + cap), with L upstream's limit (2S, or S + 1 GiB from 1 GiB), B the input bound and k = 6 (11 for multipart).
  • Request level (PHP 7.1+). The whole request is armed from the end of appsec's RINIT to the start of its RSHUTDOWN. A pending error is raised at the next VM interrupt, the way max_execution_time is delivered. Hard limit: max(L, S + 8 MiB).
  • Above a hard limit the process ends with _exit(137).

Handler lifecycle. Install once at a fixed point, chaining the previous handlers; remove at the mirrored point, only if the slot still holds ours; in between only flags change. That keeps the order LIFO with the profiler's allocation handlers in either module order. When a bailout skips our RSHUTDOWN (or request startup fails), post_deactivate or MSHUTDOWN reset the heap to plain zend_mm. The handlers also put the sentinel back whenever something writes the engine limit directly (e.g. php_error_cb() before every fatal) and follow runtime memory_limit changes. Below 8.4, gc_mem_caches() is wrapped as the profiler does.

CVE-2026-109619. Request-owned engine globals that upstream resets are reset in post_deactivate; the last-error strings are released from a sapi_module.deactivate wrapper; session upload progress is off during the startup POST parse.

Gating.

  • On only on an unpatched runtime (_femalloc does not resolve), per DD_APPSEC_PHP2SHELL_MITIGATION (auto by default; on overrides only the patch check), and following appsec's own state (RINIT, remote config).
  • No hook at all when the configuration at startup rules it out (appsec or the tracer explicitly disabled, mode off, auto on a patched runtime) or under the cli, phpdbg and embed SAPIs.
  • Two metrics (soft-deferred, tagged with its phase, and hard-exit) are flushed synchronously to the sidecar; that needs two small exports in ext/, in a separate commit.

Code layout. appsec/src/extension/php2shell/ holds one unit per function: the out-of-memory guard (handlers, limits, deferral; the allocation hot path stays in this one file), the startup-parse, request_parse_body() and runtime protections, the stale-state resets and the activation policy. Each owns its hooks, state, MINFO rows and testing functions. php2shell_mitigation.c is a facade: lifecycle sequencing, the design overview and the coverage list, where the remaining gaps are documented by consequence.

Commits:

  1. docs: libc++-21 for the appsec native tests (matches CI).
  2. ext: exports so another extension can flush telemetry to the sidecar early.
  3. appsec: the mitigation, its tests, and a CI job for the profiler integration test.

Testing

All on the final tree (aae407b23, whose code is identical to the tested tree; the last amends only changed comments), on amd64:

  • Full appsec .phpt suite on the 22 cells of the CI job "test appsec extension" (debug 7.0–8.5, debug-zts 7.0–7.3, debug-zts-asan 7.4–8.5): 0 failures. The pass counts are identical to those before the code was split into units.
  • Integration (Php2ShellMitigationTests, Php2ShellProfilerTests with -PwithProfiler) on 8.3 and 8.5: all pass. The profiler test runs php-fpm with allocation profiling on, in both module orders, and checks deferrals (runtime, startup parse, and request_parse_body() on 8.4+), worker survival across requests, the profiler's own log, and the metric phase tags. It needs the profiler build, so it runs in a dedicated CI job instead of the regular ones.
  • Lint (clang-format/clang-tidy 21.1.8) clean; Alpine/musl GCC -O2 -Werror builds on 7.0, 7.1 and 8.3.
  • Red/green: each regression test added during review failed on the code before its fix and passed after it. Examples: profiler-stacking worker crashes, memory_limit = PHP_INT_MAX, a crash at module shutdown after a failed request startup, and the 7.1 build.

🤖 Generated with Claude Code

cataphract and others added 2 commits October 23, 2026 12:03
The appsec native test instructions in .claude/ci/appsec-native-tests.md
told readers to install libc++-17-dev and libc++abi-17-dev in the
bookworm-11 CI images. Those packages no longer exist there, so the
documented apt-get step exits with status 100 and the build never
starts.

Use the -21 packages that .gitlab/generate-appsec.php installs in CI,
and the matching clang-tidy-21 name in the notes about the lint job.
The troubleshooting note now names the CI generator as the place to
look up the version, so the instructions are easier to keep in step
with the next bump.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tracer flushes buffered user-request telemetry metrics when a
request finishes, after registering the telemetry application
(service/env) on the sidecar connection in its own RINIT. An extension
that needs to report something at a point the tracer never reaches --
during request startup, before the tracer's RINIT, or right before the
process calls _exit() -- has no way to get its metric points delivered:
the request-finish flush does not run, and a plain buffer flush made
before the application is registered is dropped by the sidecar ("No
application found").

The appsec PHP2Shell mitigation (next commit) is such a case: it can
end a request during startup with a deferred out-of-memory error, or
terminate the worker with _exit(137) when the hard memory limit is
crossed, and its counters would otherwise be lost.

Export two functions so that another extension can resolve them with
dlsym() and flush synchronously to the sidecar, which outlives the
worker:

- datadog_telemetry_commit_user_request_metrics() already existed; it
  is now DATADOG_PUBLIC. The extension builds with hidden visibility,
  so without that a dlsym() from appsec cannot find it.
- datadog_sidecar_ensure_telemetry_application() is new. It registers
  (or refreshes) the telemetry application on the sidecar connection
  with the configured service, env and version, as
  datadog_sidecar_rinit() does. set_application does nothing when the
  application is unchanged, so calling it again later in the request
  is harmless.

Both are listed in ddtrace-extension.sym. The flush is a bounded,
blocking IPC write; the only intended callers are rare failure paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@datadog-prod-us1-4

datadog-prod-us1-4 Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Pipelines  Tests

✨ Unblock PR with BitsAI

❌ Errors

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 3 Pipeline jobs failed

DataDog/apm-reliability/dd-trace-php | validate_supported_configurations_v2_local_file — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

DataDog/apm-reliability/dd-trace-php | merge-gate

View more details · View in GitLab

DataDog/apm-reliability/dd-trace-php | publish docker image for system tests

View more details · View in GitLab

ℹ️ Info

No other issues found (see more)

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 79.12%
• Overall Coverage: 74.84% (-1.67%)

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: aae407b | Docs | View more details | Give us feedback!

@cataphract
cataphract force-pushed the glopes/appsec-oom-soft-limit branch 2 times, most recently from ef9d585 to 89bfe1e Compare October 27, 2026 10:37
@pr-commenter

pr-commenter Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Benchmarks [ appsec ]

Benchmark execution time: 2026-10-28 16:47:10

Comparing candidate commit aae407b in PR branch glopes/appsec-oom-soft-limit with baseline commit 7efe423 in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 2 performance regressions! Performance is the same for 10 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:LaravelBench/benchLaravelOverhead-appsec

  • 🟥 execution_time [+286.622µs; +634.978µs] or [+2.052%; +4.546%]

scenario:WordPressBench/benchWordPressOverhead-appsec

  • 🟥 execution_time [+1.925ms; +2.360ms] or [+5.412%; +6.637%]

@bwoebi

bwoebi commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Fwiw: this does nothing if has_prev, and the profiler does install its own forwarding handlers.

@pr-commenter

pr-commenter Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Benchmarks [ tracer ]

Benchmark execution time: 2026-10-28 17:34:25

Comparing candidate commit aae407b in PR branch glopes/appsec-oom-soft-limit with baseline commit 7efe423 in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 3 performance regressions! Performance is the same for 190 metrics, 1 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:ComposerTelemetryBench/benchTelemetryParsing

  • 🟥 execution_time [+0.882µs; +1.718µs] or [+5.800%; +11.305%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟥 execution_time [+3.474µs; +4.686µs] or [+3.091%; +4.168%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟥 execution_time [+5.611µs; +7.169µs] or [+5.204%; +6.649%]

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:LaravelBench/benchLaravelDdprof-opcache

  • unstable execution_time [-215.558µs; +1363.398µs] or [-1.523%; +9.631%]

@cataphract
cataphract force-pushed the glopes/appsec-oom-soft-limit branch from 89bfe1e to cc09c06 Compare October 27, 2026 11:49
@bwoebi

bwoebi commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Is there a specific reason you went with an attempt at emulating the full fix partially, rather than just going with the emulation of the minimal fix?
php/php-src@1d7dec1

With the way you emulated it, it's not possible to get an OOM during input parsing, but it does not prevent any delayed OOM, e.g. bringing heap size to 127.9M and then just nudging it over the limit at a controlled place (like an array operation in runtime code - e.g. if you simply have a wordpress installation, an attack can just whitebox that).

@cataphract
cataphract force-pushed the glopes/appsec-oom-soft-limit branch 2 times, most recently from 5ba9e92 to bedd9cf Compare October 28, 2026 15:39
CVE-2026-109618: on unpatched PHP, _zend_hash_str_add_or_update_i()
bumps a hash table's entry counters before allocating the key. If that
allocation crosses memory_limit, the engine longjmps out of the
insertion, and request shutdown later walks the counted but
uninitialised bucket as a zval. Request variables are parsed before any
userland runs and land in superglobals, which shutdown always destroys,
so the request alone reaches it; parse_str() and other char*-key
builders reach it at runtime. CVE-2026-109619: request-owned engine
globals are left pointing into the freed request heap after a fast
shutdown, and early code in the next request can read them. Upstream
fixed both in 8.2.35/8.3.36/8.4.27/8.5.12, with an engine-wide soft
limit that defers the error to a safe point, a hard limit that exits,
and resets of the stale state. Customers on older patch levels stay
exposed, and an extension cannot patch the engine, so appsec mitigates
the parts it can reach.

While the heap is armed, our Zend MM handlers (chained over any
existing ones, such as the profiler's) serve every allocation and the
engine limit is lifted to a sentinel, so no insertion is abandoned.
Crossing the soft limit (memory_limit) makes the error pending instead.
Two parses get a window of their own: the startup treat_data parse of
POST, GET and COOKIE data before RINIT, and request_parse_body() on
8.4+. In a window an input_filter wrapper drops later variables, a
php_rfc1867_callback wrapper skips later file parts at FILE_START and a
read_post wrapper ends a multipart body, so growth stops with the item
in flight; when the parse returns, the arrays are complete and the
error is raised as a normal E_ERROR under a finite limit. A window's
hard limit bounds the in-flight item: min(max(L, S + k*B + 8 MiB),
S + cap), with L upstream's own limit (2S, or S + 1 GiB from 1 GiB), B
the input bound of the parse, k = 6 per input byte, or 11 for a
multipart body, and a constant cap of max(1 GiB, 2S), or 4S for
multipart.

On PHP 7.1+ the whole request is armed too, from the end of appsec's
RINIT to the start of its RSHUTDOWN. There a pending error is raised at
the next VM interrupt, after the inner interrupt handlers ran, the way
max_execution_time is delivered, and the hard limit is max(L, S +
8 MiB). Making the affected allocations fallible, as upstream does,
would need engine changes that cannot reach unpatched runtimes. Above
any hard limit, or before a fresh huge allocation would cross it, the
process ends with _exit(137) rather than run cleanup over a runaway
heap.

The handlers follow one rule: install once at a fixed point, chaining
the previous handlers; remove at the mirrored point, only if the slot
still holds ours; in between only flags change. That keeps the order
LIFO with the profiler, which installs in RINIT and removes in
RSHUTDOWN, in either module order. Swapping handlers in and out
mid-request, or removing them where they were not installed, broke
that order and made later requests recurse. When a bailout skips our
RSHUTDOWN, post_deactivate and, after a failed request startup,
MSHUTDOWN reset the heap to plain zend_mm, because the profiler's own
fallback has already run by then. "Installed" means "in the chain":
with another extension on top we stay chained under it, we never chain
over ourselves, and before arming without installing we check the slot.
The handlers also put the sentinel back whenever something writes the
engine limit directly (php_error_cb() before every fatal, appsec's own
restore) and adopt a changed memory_limit as the new soft limit, so
runtime changes are followed however they happen; an INI on_modify hook
was dropped because it only saw changes while it sat on top of the
chain. Below 8.4, gc_mem_caches() is wrapped as the profiler does,
since zend_mm_gc() does nothing on a heap with custom handlers.

For CVE-2026-109619, post_deactivate resets the request-owned EG/PG
fields, the object store and the superglobal slots that upstream
resets; a sapi_module.deactivate wrapper releases the last-error
strings after every extension's post_deactivate; and session upload
progress is turned off for the startup POST parse, so the session
callback cannot decode session data before RINIT.

Every hook is captured once, from the zend_extension startup hook after
all MINITs. A read-only check per request logs once and shows in MINFO
if another extension displaces one, though it cannot tell a chaining
wrapper from a replacement. No hook is installed at all when the
configuration seen at startup rules the mitigation out: appsec or the
tracer explicitly disabled, the mode off, the mode auto on a patched
runtime, or the cli, phpdbg and embed SAPIs, which carry no
attacker-controlled request data. A php-fpm pool therefore cannot
re-enable what the master disabled; we accept that rather than
intercept the parses of every process that will never use it.
Otherwise the mitigation is gated on an unpatched runtime (the fix's
_femalloc symbol does not resolve; on forces it on a patched one), on
DD_APPSEC_PHP2SHELL_MITIGATION (auto by default), and on a process-wide
flag that follows appsec's own state, from RINIT and remote config. Two
metrics, the deferred error (tagged with the phase: startup,
request_parse_body or runtime) and the hard exit, are flushed
synchronously to the sidecar, because the request dies in startup or
the process exits before the normal flush.

The code is split by function under appsec/src/extension/php2shell/:
the out-of-memory guard (handlers, limits, deferral), the startup-parse,
request_parse_body() and runtime protections, the stale-state resets
and the activation policy. Each unit owns its hooks, state, MINFO rows
and testing functions; php2shell_mitigation.c is a facade sequencing
the lifecycle and holding the design overview and the coverage list.
The guard's per-thread state and the allocation hot path stay in one
file, so no allocation crosses a unit. Every unit includes ddappsec.h,
so that on ZTS builds of PHP 8 the engine's TSRMLS_CACHE keeps appsec's
local-dynamic TLS model and ddappsec.so is not marked STATIC_TLS.

Coverage limits are documented in the source, grouped by consequence:
what stays unprotected (FrankenPHP's per-request re-import,
USE_ZEND_ALLOC=0, OS allocation refusals, code outside appsec's
RINIT..RSHUTDOWN), where the gate does not follow the expected
configuration, which cases end in a 137 rather than the deferred fatal
(the caps, very large max_input_vars, an expanding filter.default, a
memory_limit lowered below usage on old patch releases, a skipped
RSHUTDOWN on 7.x), and the observable differences from stock PHP.

Tests cover the windows and the request level in php-cgi and php -S
children (the mitigation is off under the cli test process), red on
the code before each fix; a p2s_stub test extension exercises the
lifecycle fallbacks (bailouts in RINIT and RSHUTDOWN, a neighbour
chaining over our handlers). The integration tests check the metrics,
and a dedicated CI job runs the mitigation next to the profiler with
allocation profiling on, across requests and in both module orders.

Alternatives considered and rejected: arming only around insertions
(no hook marks the end of a registration); arming individual runtime
functions (no closed list, and the VM's own array writes are
unreachable); hot-patching zend_hash (per-version reimplementations in
live code, blocked by W^X policies); a separate opt-in mode for the
request level; and honouring per-directory datadog.appsec.enabled per
request, which appsec itself does not do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cataphract
cataphract force-pushed the glopes/appsec-oom-soft-limit branch from bedd9cf to aae407b Compare October 28, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants