Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harden safety around dead and badly typed Gd<T> instances #546

Merged
merged 3 commits into from
Dec 27, 2023

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Dec 27, 2023

Fixes a wide range of UB in scenarios when interacting with Gd<T> instances that are either dead or wrongly typed due to the #23 DerefMut exploit. The latter is done by tracking the runtime type in each Gd<T> object as debug information.

Release mode has the type check disabled, but the object validity one enabled. This might be worth a separate discussion in the future, maybe if performance impact is measured.

Scenarios covered:

  • passing Gd<T> to godot::engine APIs
  • returning Gd<T> from #[func]
  • converting via ToGodot trait
  • upcast() and cast()
  • bind() and bind_mut()
  • clone()
  • free() on wrong type
  • free() that panics when there is already a panic unwind in progress

There might be more, but this significantly improves safety when working with objects, to the point that it catches even deliberate abuse.

Add ObjectRtti type to track object identity and type information.
In Debug mode, verify inheritance relationship between T (dynamic type) and Gd<T> (static type).

Also address UB with dead objects being passed to engine APIs.
@Bromeon Bromeon added bug c: core Core components c: ffi Low-level components and interaction with GDExtension API ub Undefined behavior labels Dec 27, 2023
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-546

@Bromeon Bromeon added this pull request to the merge queue Dec 27, 2023
Merged via the queue into master with commit c92de58 Dec 27, 2023
17 checks passed
@Bromeon Bromeon deleted the bugfix/subtype-ub branch December 27, 2023 22:06
@Bromeon Bromeon mentioned this pull request Feb 9, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: core Core components c: ffi Low-level components and interaction with GDExtension API ub Undefined behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants