Skip to content

Fix box2d physics use-after-free crashes#905

Open
zero-meta wants to merge 4 commits into
coronalabs:masterfrom
labolado:fix/physics-uaf-crashes
Open

Fix box2d physics use-after-free crashes#905
zero-meta wants to merge 4 commits into
coronalabs:masterfrom
labolado:fix/physics-uaf-crashes

Conversation

@zero-meta
Copy link
Copy Markdown
Contributor

Fix physics use-after-free crashes

Summary

Fix multiple use-after-free vulnerabilities in the Box2D physics binding that cause SIGSEGV crashes in production (Our Google Play App).

  • StopWorld 3-phase cleanup: Replace per-body DestroyBody loop with bulk cleanup to eliminate double-free when gear joints have m_bodyA == m_bodyB (both edges in same body's joint list)
  • StepWorld offscreen fix: Remove GetParent() guard in ~DisplayObjectExtensions so offscreen display objects (snapshot.group, canvas texture cache) unconditionally clear body->SetUserData(NULL), preventing dangling pointer dereference on next frame. The same as this PR: (removed) #894
  • Finalizer ownership check: Add bidirectional joint->GetUserData() == wrapper guard before writing to b2Joint userdata in Lua GC finalizer
  • Pre-invalidate joint wrappers: Invalidate all attached joint UserdataWrappers immediately in ~DisplayObjectExtensions, closing the GC timing window between removeSelf and StepWorld's deferred DestroyBody

Crash signatures (from production)

[libcorona.so] b2Fixture::Destroy(b2BlockAllocator*) SIGSEGV
[libcorona.so] PhysicsJoint::Finalizer                SIGSEGV
[Corona Simulator] b2Joint::Destroy + 32               SIGSEGV at 0x0

Prior fix context

This PR #858 (commit c8c3bd1e) ("Core: fix box2d joint use after free when world deleted") added DestroyBody(body) to the StopWorld loop. This fixed the original bug where delete fWorld bulk-freed all joint memory without calling SayGoodbye, leaving joint wrappers dangling for the GC Finalizer to crash on. With DestroyBody, each joint gets SayGoodbyewrapper->Invalidate() before being freed, making the Finalizer safe for all normal joints.

However, DestroyBody introduced a new deterministic crash for gear joints: b2GearJoint has m_bodyA == m_bodyB == ground, so both joint edges are in the same body's list, and DestroyBody(ground) double-frees the gear joint.

This PR's 3-phase StopWorld solves both problems simultaneously: no DestroyBody (avoids gear joint double-free), but manually iterates the joint list to wrapper->Invalidate() (replaces SayGoodbye's role).

Gear joint deterministic crash — test code

The gear joint crash is 100% reproducible. b2GearJoint overrides m_bodyA/m_bodyB to the sub-joints' other bodies, causing both joint edges to register in the same body's list (ground). When DestroyBody(ground) iterates this list, it frees the gear joint via edgeB, then follows the prefetched next pointer to edgeA — which is now freed memory.

Paste into main.lua and run — crashes immediately on physics.stop():

local physics = require("physics")
physics.start()
physics.setGravity(0, 9.8)

-- Bodies
local ground = display.newRect(160, 450, 320, 20)
physics.addBody(ground, "static")

local wheelA = display.newCircle(100, 300, 30)
physics.addBody(wheelA, "dynamic", { radius = 30, density = 1.0 })

local wheelB = display.newCircle(220, 300, 30)
physics.addBody(wheelB, "dynamic", { radius = 30, density = 1.0 })

-- Two pivot joints anchored to ground
local pivotA = physics.newJoint("pivot", wheelA, ground, 100, 300)
local pivotB = physics.newJoint("pivot", wheelB, ground, 220, 300)

-- Gear joint links the two pivots — internally sets m_bodyA = m_bodyB = ground
local gearJoint = physics.newJoint("gear", wheelA, wheelB, pivotA, pivotB, 1.5)

-- Stop physics after a short delay → StopWorld → DestroyBody(ground) → SIGSEGV
timer.performWithDelay(500, function()
    physics.stop()  -- deterministic crash: b2Joint::Destroy + 32, SIGSEGV at 0x0
end)

zero-meta and others added 4 commits April 12, 2026 17:16
Replace per-body `DestroyBody` loop in `StopWorld()` with 3-phase bulk
cleanup to eliminate use-after-free when `DestroyBody` calls `DestroyJoint`
on joints whose memory has already been freed by a previous `DestroyBody`.

The crash is deterministic with gear joints (`m_bodyA == m_bodyB` causes
both joint edges to register in the same body's list, so `DestroyJoint`
corrupts the list being iterated) and probabilistic with normal joints
(depends on GC timing and `b2BlockAllocator` memory reuse zeroing the
vtable).
Add bidirectional ownership check before writing to `b2Joint` userdata in
the Lua GC finalizer. Only clear the joint's userdata if it still points
back to this wrapper (`joint->GetUserData() == wrapper`). Without the
check, the finalizer could write to memory that `b2BlockAllocator` has
already freed and reused.
~DisplayObjectExtensions previously checked GetParent() before clearing
b2Body userData. GetParent() returns NULL for objects with
IsRenderedOffScreen flag (snapshot.group, canvas texture cache groups),
causing SetUserData(NULL) to be skipped. This leaves dangling pointers
in the Box2D world body list, which StepWorld dereferences on the next
frame, causing SIGSEGV.

The fix removes the parent dependency and unconditionally clears userData.
This is safe because SetUserData(NULL) has no side effects and the body
is lazily destroyed by StepWorld when it finds NULL userData.
When a display object with a physics body is destroyed (via `removeSelf`
or group removal), invalidate all attached joint `UserdataWrapper`s
immediately in the destructor, before setting `body->SetUserData(NULL)`.

This closes the timing window between `removeSelf` and `StepWorld`'s
deferred `DestroyBody`: without pre-invalidation, a GC cycle in that
window could run `PhysicsJoint::Finalizer` on a still-valid wrapper
pointing to a joint about to be destroyed.
@zero-meta zero-meta requested a review from Shchvova as a code owner April 12, 2026 11:50
@zero-meta zero-meta changed the title Fix box2d physics use after free crashes Fix box2d physics use-after-free crashes Apr 12, 2026
@labolado labolado deleted the fix/physics-uaf-crashes branch May 7, 2026 10:27
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