Possible use-after-free of the Go backing array of extensions retained by register_extensions
registerExtensions hands the backing array of the Go slice extensions to C, which
stores it in a static global, and then immediately sets extensions = nil. The array is
read later, from PHP startup, by which point nothing in Go references it and the garbage
collector is free to reclaim it.
ext.go:20
func registerExtensions() {
if len(extensions) == 0 {
return
}
registerOnce.Do(func() {
C.register_extensions((**C.zend_module_entry)(unsafe.Pointer(&extensions[0])), C.int(len(extensions)))
extensions = nil
})
}
frankenphp.c:1839 stores the pointer rather than copying:
void register_extensions(zend_module_entry **m, int len) {
modules = m;
modules_len = len;
original_php_register_internal_extensions_func =
php_register_internal_extensions_func;
php_register_internal_extensions_func = register_internal_extensions;
}
modules is a file-static (frankenphp.c:1817) and is only dereferenced later, from the
callback that was just installed (frankenphp.c:1821):
int register_internal_extensions(void) {
if (original_php_register_internal_extensions_func != NULL &&
original_php_register_internal_extensions_func() != SUCCESS) {
return FAILURE;
}
for (int i = 0; i < modules_len; i++) {
if (zend_register_internal_module(modules[i]) == NULL) {
return FAILURE;
}
}
So the sequence is:
registerExtensions passes &extensions[0], a pointer into Go-allocated memory.
register_extensions saves it in modules and returns. C now holds a Go pointer past
the end of the cgo call, which the cgo pointer-passing rules do not allow.
extensions = nil drops the only Go reference to that array.
- PHP later calls
register_internal_extensions, which reads modules[i].
Between 3 and 4 the array is unreachable from Go, so it can be collected. The elements
themselves are fine, since RegisterExtension takes unsafe.Pointer values that live in
C memory; it is the array of pointers that is Go-allocated.
I have not produced a crash from this, and in practice the window may be short enough that
collection does not happen. The retention itself is unconditional though, and dropping the
reference on the next line removes the only thing that would have kept it alive.
runtime.Pinner is already used elsewhere in the project (phpthread.go:19).
Fix: pin the array for as long as C holds it, and keep the slice alive.
var extPinner runtime.Pinner
registerOnce.Do(func() {
extPinner.Pin(&extensions[0])
C.register_extensions((**C.zend_module_entry)(unsafe.Pointer(&extensions[0])), C.int(len(extensions)))
})
Dropping extensions = nil and unpinning once register_internal_extensions has run
would be enough. Copying the array into C memory with C.malloc before the call avoids
the question entirely.
If you could credit me as a reporter for my contributions to security advisory I will be thankful.
Possible use-after-free of the Go backing array of
extensionsretained byregister_extensionsregisterExtensionshands the backing array of the Go sliceextensionsto C, whichstores it in a static global, and then immediately sets
extensions = nil. The array isread later, from PHP startup, by which point nothing in Go references it and the garbage
collector is free to reclaim it.
ext.go:20
frankenphp.c:1839 stores the pointer rather than copying:
modulesis a file-static (frankenphp.c:1817) and is only dereferenced later, from thecallback that was just installed (frankenphp.c:1821):
So the sequence is:
registerExtensionspasses&extensions[0], a pointer into Go-allocated memory.register_extensionssaves it inmodulesand returns. C now holds a Go pointer pastthe end of the cgo call, which the cgo pointer-passing rules do not allow.
extensions = nildrops the only Go reference to that array.register_internal_extensions, which readsmodules[i].Between 3 and 4 the array is unreachable from Go, so it can be collected. The elements
themselves are fine, since
RegisterExtensiontakesunsafe.Pointervalues that live inC memory; it is the array of pointers that is Go-allocated.
I have not produced a crash from this, and in practice the window may be short enough that
collection does not happen. The retention itself is unconditional though, and dropping the
reference on the next line removes the only thing that would have kept it alive.
runtime.Pinneris already used elsewhere in the project (phpthread.go:19).Fix: pin the array for as long as C holds it, and keep the slice alive.
Dropping
extensions = niland unpinning onceregister_internal_extensionshas runwould be enough. Copying the array into C memory with
C.mallocbefore the call avoidsthe question entirely.
If you could credit me as a reporter for my contributions to security advisory I will be thankful.