Use-after-free: class name of an observed static method is released without a reference being taken (#[WithSpan] and hook())

[terev]

Describe your environment

• PHP 8.4.24 (php:8.4-cli-alpine), also reproduced on 8.4.21 (Alpine php84)
• ext-opentelemetry 1.2.1 (current on PECL), 1.4.0, and main @ 991d3279
• open-telemetry/api 1.8.0
• opcache enabled, opentelemetry.attr_hooks_enabled=1
• linux/arm64 and linux/amd64, in Docker

Checked against master: still reproduces. The code involved is unchanged between 1.2.1, 1.4.0 and
main.

This is not specific to #[WithSpan]. Any observed static method hits it, including one observed
through an OpenTelemetry\Instrumentation\hook() registration with no attribute anywhere and no SDK
in play. The hook case also reproduces with opentelemetry.attr_hooks_enabled=0, so that setting
does not contain it.

Steps to reproduce

Self contained repro Dockerfile, everything inlined

# syntax=docker/dockerfile:1.7
FROM php:8.4-cli-alpine

COPY --from=composer:2 /usr/bin/composer /usr/bin/composer

ARG OTEL_TAG=1.4.0
RUN apk add --no-cache --virtual .build-deps $PHPIZE_DEPS curl tar \
 && curl -sSL "https://github.com/open-telemetry/opentelemetry-php-instrumentation/archive/refs/tags/${OTEL_TAG}.tar.gz" -o /tmp/otel.tgz \
 && tar -xzf /tmp/otel.tgz -C /tmp \
 && cd "/tmp/opentelemetry-php-instrumentation-${OTEL_TAG}/ext" \
 && phpize && ./configure && make -j4 && make install \
 && docker-php-ext-enable opentelemetry \
 && docker-php-ext-install opcache \
 && apk del .build-deps

RUN printf 'opcache.enable_cli=1\nopentelemetry.attr_hooks_enabled=1\n' \
      > /usr/local/etc/php/conf.d/otel.ini

WORKDIR /repro

COPY <<'EOF' /repro/composer.json
{ "require": { "open-telemetry/api": "^1.8" } }
EOF

COPY <<'EOF' /repro/AttrTarget.php
<?php

namespace ReproNs;

use OpenTelemetry\API\Instrumentation\WithSpan;

class AttrTarget
{
    #[WithSpan]
    public static function pingStatic(): int
    {
        return 1;
    }

    #[WithSpan]
    public function pingInstance(): int
    {
        return 1;
    }
}
EOF

COPY <<'EOF' /repro/HookTarget.php
<?php

namespace ReproNs;

class HookTarget
{
    public static function pingStatic(): int
    {
        return 1;
    }

    public function pingInstance(): int
    {
        return 1;
    }
}
EOF

COPY <<'EOF' /repro/attr_static.php
<?php

require __DIR__ . '/vendor/autoload.php';
require __DIR__ . '/AttrTarget.php';

\ReproNs\AttrTarget::pingStatic();
echo "survived\n";
EOF

COPY <<'EOF' /repro/attr_instance.php
<?php

require __DIR__ . '/vendor/autoload.php';
require __DIR__ . '/AttrTarget.php';

(new \ReproNs\AttrTarget())->pingInstance();
echo "survived\n";
EOF

COPY <<'EOF' /repro/hook_static.php
<?php

require __DIR__ . '/vendor/autoload.php';
require __DIR__ . '/HookTarget.php';

\OpenTelemetry\Instrumentation\hook(
    \ReproNs\HookTarget::class,
    'pingStatic',
    pre: static function (mixed $target, array $params, ?string $class, string $function): void {},
    post: static function (): void {},
);

\ReproNs\HookTarget::pingStatic();
echo "survived\n";
EOF

COPY <<'EOF' /repro/hook_instance.php
<?php

require __DIR__ . '/vendor/autoload.php';
require __DIR__ . '/HookTarget.php';

\OpenTelemetry\Instrumentation\hook(
    \ReproNs\HookTarget::class,
    'pingInstance',
    pre: static function (mixed $target, array $params, ?string $class, string $function): void {},
    post: static function (): void {},
);

(new \ReproNs\HookTarget())->pingInstance();
echo "survived\n";
EOF

# Both the declaring file and the calling file. Leaving the caller out lets the literal
# class name there be interned, which hides the fault.
COPY <<'EOF' /repro/blacklist.txt
/repro/AttrTarget.php
/repro/HookTarget.php
/repro/attr_static.php
/repro/attr_instance.php
/repro/hook_static.php
/repro/hook_instance.php
EOF

RUN composer install --no-interaction --quiet

COPY <<'EOF' /usr/local/bin/repro
#!/bin/sh
printf 'PHP %s, ext-opentelemetry %s\n\n' \
    "$(php -r 'echo PHP_VERSION;')" "$(php -r 'echo phpversion("opentelemetry");')"

run() {
    label=$1
    script=$2
    interned=$3

    if [ "$interned" = "yes" ]; then
        USE_ZEND_ALLOC=0 php "/repro/$script" > /dev/null 2>&1
    else
        USE_ZEND_ALLOC=0 php -d opcache.blacklist_filename=/repro/blacklist.txt "/repro/$script" > /dev/null 2>&1
    fi

    code=$?
    if [ "$code" -eq 0 ]; then
        result="survived"
    else
        result="CRASHED"
    fi
    printf '%-28s class name interned: %-3s  exit=%-4s %s\n' "$label" "$interned" "$code" "$result"
}

run '#[WithSpan] static'  attr_static.php   yes
run '#[WithSpan] static'  attr_static.php   no
run 'hook() static'       hook_static.php   yes
run 'hook() static'       hook_static.php   no
printf '\ncontrols, same conditions but not static:\n'
run '#[WithSpan] instance' attr_instance.php no
run 'hook() instance'      hook_instance.php no
EOF

RUN chmod +x /usr/local/bin/repro

ENTRYPOINT ["/usr/local/bin/repro"]

Build another version with --build-arg OTEL_TAG=1.2.1.

docker build -t otel-static-repro .
docker run --rm otel-static-repro

Each case runs the same script twice; the only difference is whether opcache is allowed to store the
files, which decides whether ce->name is interned. USE_ZEND_ALLOC=0 is set only to make the
failure immediate rather than deferred. The blacklist has to name the calling file as well as the
declaring one, otherwise the literal class name in the caller is interned and nothing fails.

What is the expected behavior?

All six cases print survived and exit 0. Whether opcache happens to have interned a class name
should not affect whether observing a static method is safe, and it should not matter whether the
observation came from an attribute or from hook().

What is the actual behavior?

Both static cases segfault on the first call, reproducibly, on 1.2.1 and 1.4.0 alike. Instance
methods are unaffected either way:

PHP 8.4.24, ext-opentelemetry 1.4.0

#[WithSpan] static           class name interned: yes  exit=0    survived
#[WithSpan] static           class name interned: no   exit=139  CRASHED
hook() static                class name interned: yes  exit=0    survived
hook() static                class name interned: no   exit=139  CRASHED

controls, same conditions but not static:
#[WithSpan] instance         class name interned: no   exit=0    survived
hook() instance              class name interned: no   exit=0    survived

#0  get_meta (p=0xffffe00b2420) at src/malloc/mallocng/meta.h:141
#1  __libc_free (p=0xffffe00b2420) at src/malloc/mallocng/free.c:105
#2  zval_ptr_dtor_nogc (...) at Zend/zend_variables.h:36
#3  observer_end (...) at otel_observer.c:869
#4  zend_observer_fcall_end_prechecked ()

Cause and a fix

func_get_this_or_called_scope takes no reference on the static branch, while the instance branch
does:

if (execute_data->func->op_array.fn_flags & ZEND_ACC_STATIC) {
    zend_class_entry *called_scope = zend_get_called_scope(execute_data);
    ZVAL_STR(zv, called_scope->name);      // no reference taken
} else {
    zend_object *this = zend_get_this_object(execute_data);
    ZVAL_OBJ_COPY(zv, this);               // reference taken
}

Both observer_begin and observer_end fill params[0] from it and then release every param with
zval_dtor, so each observed call drops a reference it never took. That code is shared by attribute
handlers and hook() registrations, which is why both routes fail and why disabling
attr_hooks_enabled does not help. Interned class names absorb the underflow, so the failure only
appears once opcache has not interned the name, which makes it look intermittent and environmental.

Building the same image with the static branch changed to ZVAL_STR_COPY(zv, called_scope->name);
makes all six cases pass.

Additional context

Related but distinct: #2002 (attribute arguments) and #2031
(#[SpanAttribute] parameters) are the same class of defect at other sites. 1.4.0 contains the
#2002 fix and still reproduces the above. #1846 reports matching symptoms.