From 6764338e096f5ca72fa4f7750f85d8ed5d9d5a65 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Sun, 22 Sep 2024 08:29:30 +0100 Subject: [PATCH] Fix physics platform behaviour regression Lifetime checks for stored `RIDs` for collision objects assumed they had valid `object_ids`. It turns out that some are not derived from `Object` and thus checking `ObjectDB` returns false for some valid `RIDs`. To account for this we only perform lifetime checks on valid `object_ids`. --- scene/3d/physics/character_body_3d.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scene/3d/physics/character_body_3d.cpp b/scene/3d/physics/character_body_3d.cpp index dda3ea9cca2..e3815e82190 100644 --- a/scene/3d/physics/character_body_3d.cpp +++ b/scene/3d/physics/character_body_3d.cpp @@ -60,8 +60,13 @@ bool CharacterBody3D::move_and_slide() { // We need to check the platform_rid object still exists before accessing. // A valid RID is no guarantee that the object has not been deleted. - if (ObjectDB::get_instance(platform_object_id)) { - //this approach makes sure there is less delay between the actual body velocity and the one we saved + + // We can only perform the ObjectDB lifetime check on Object derived objects. + // Note that physics also creates RIDs for non-Object derived objects, these cannot + // be lifetime checked through ObjectDB, and therefore there is a still a vulnerability + // to dangling RIDs (access after free) in this scenario. + if (platform_object_id.is_null() || ObjectDB::get_instance(platform_object_id)) { + // This approach makes sure there is less delay between the actual body velocity and the one we saved. bs = PhysicsServer3D::get_singleton()->body_get_direct_state(platform_rid); }