Skip to content

Commit 1d139c5

Browse files
profiling: guard against NULL file in timeline error observer
ddog_php_prof_zend_error_observer dereferenced the error file pointer without a NULL check. When an uncaught exception with a NULL file location is reported as a fatal error, the engine calls the error observer with file=NULL (location "Unknown"); on PHP 8.0 the observer receives a raw C string and CStr::from_ptr(NULL) segfaulted in strlen(). This crashed any PHP 8.0 CLI process with the profiler loaded and timeline enabled (the default) whenever such a fatal error occurred, e.g. the upstream tests Zend/tests/bug50005.phpt and Zend/tests/bug64821.3.phpt (both build an exception with a NULL file and throw it uncaught). NULL-guard the file pointer (PHP 8.1+ already handled NULL via the ZendString path). Add a regression test profiling/tests/phpt/timeline_fatal_error_null_filename.phpt. Verified the crash is gone on 8.0 ZTS and no regression on 8.4.
1 parent 07b1b58 commit 1d139c5

2 files changed

Lines changed: 61 additions & 4 deletions

File tree

profiling/src/timeline.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,12 +245,23 @@ unsafe extern "C" fn ddog_php_prof_zend_error_observer(
245245
return;
246246
}
247247

248+
// The engine passes a NULL file for fatal errors with no active file
249+
// location, e.g. an uncaught exception reported via zend_exception_error
250+
// (`zend_error_va(..., file=NULL, ...)`). CStr::from_ptr(NULL) would
251+
// strlen(NULL) and segfault, so guard against it. See
252+
// Zend/tests/bug50005.phpt and Zend/tests/bug64821.3.phpt.
248253
#[cfg(zend_error_observer_80)]
249-
let filename_str = unsafe { core::ffi::CStr::from_ptr(file) };
254+
let filename = if file.is_null() {
255+
String::new()
256+
} else {
257+
unsafe { core::ffi::CStr::from_ptr(file) }
258+
.to_string_lossy()
259+
.into_owned()
260+
};
250261
#[cfg(not(zend_error_observer_80))]
251-
let filename_str = unsafe { zai_str_from_zstr(file.as_mut()) };
252-
253-
let filename = filename_str.to_string_lossy().into_owned();
262+
let filename = unsafe { zai_str_from_zstr(file.as_mut()) }
263+
.to_string_lossy()
264+
.into_owned();
254265

255266
let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
256267
if let Some(profiler) = Profiler::get() {
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
[profiling] fatal error with a NULL file location must not crash the timeline error observer
3+
--DESCRIPTION--
4+
Regression test for a NULL pointer dereference in the timeline error observer.
5+
6+
When an uncaught exception whose `file` property is NULL is reported, the
7+
engine calls the error notification with file=NULL (location "Unknown"). The
8+
profiler's timeline error observer used to call CStr::from_ptr(NULL) (PHP 8.0,
9+
where the observer receives a raw C string), which segfaulted in strlen().
10+
11+
Reproduces the upstream Zend/tests/bug50005.phpt and bug64821.3.phpt crashes
12+
that only triggered with the profiler loaded and timeline enabled.
13+
--SKIPIF--
14+
<?php
15+
if (!extension_loaded('datadog-profiling'))
16+
echo "skip: test requires Datadog Continuous Profiler\n";
17+
// The crash is specific to PHP 8.0: the error observer receives a raw C
18+
// string there (NULL-unsafe), while 8.1+ gets a NULL-safe zend_string. Also,
19+
// Exception::$file became a typed `string` in 8.1, so `$this->file = null`
20+
// throws a TypeError and can no longer produce a NULL-file fatal at all.
21+
if (PHP_VERSION_ID < 80000 || PHP_VERSION_ID >= 80100)
22+
echo "skip: NULL-file fatal error observer crash is specific to PHP 8.0\n";
23+
?>
24+
--ENV--
25+
DD_PROFILING_ENABLED=yes
26+
DD_PROFILING_TIMELINE_ENABLED=yes
27+
DD_PROFILING_ALLOCATION_ENABLED=no
28+
DD_PROFILING_EXCEPTION_ENABLED=no
29+
DD_PROFILING_LOG_LEVEL=off
30+
--FILE--
31+
<?php
32+
33+
class a extends exception {
34+
public function __construct() {
35+
$this->file = null;
36+
}
37+
}
38+
39+
throw new a;
40+
41+
?>
42+
--EXPECTF--
43+
Fatal error: Uncaught a in :%d
44+
Stack trace:
45+
#0 {main}
46+
thrown in Unknown on line %d

0 commit comments

Comments
 (0)