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

Don't use a reference for RaycastState::m_pointabilities #14376

Merged
merged 1 commit into from Feb 17, 2024

Conversation

Desour
Copy link
Member

@Desour Desour commented Feb 16, 2024

Lua raycasts init the pointabilities with nullopt:
LuaRaycast *o = new LuaRaycast(core::line3d<f32>(pos1, pos2), objects, liquids, std::nullopt);
Which of course has a too short lifetime.

(If you want to reduce copies, shared_ptr<const Pointabilities> might work.)

Bug introduced by #13992 (after my review :3).

To do

This PR is a Ready for Review.

How to test

  • Compile with asan.
  • Spawn the devtest testentities:selectionbox entity.

@Desour Desour added Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines Bugfix 🐛 PRs that fix a bug labels Feb 16, 2024
@grorp
Copy link
Member

grorp commented Feb 16, 2024

You can consider it offtopic for this PR, but: We should allow Lua raycasts to specify pointabilities as well. This would be useful for reproducing the results of client-side raycasts.

@appgurueu
Copy link
Contributor

How about just referencing a nullopt with static lifetime (may be private to the function)?

@Desour
Copy link
Member Author

Desour commented Feb 17, 2024

How about just referencing a nullopt with static lifetime (may be private to the function)?

Would be possible, but unnecessarily complex imo. Also won't work if a mod provided pointabilities will be given to the lua raycast.

@Desour Desour merged commit 1e316a9 into minetest:master Feb 17, 2024
13 checks passed
@Desour Desour deleted the fix_raycast_pointabilities_uaf branch February 17, 2024 17:36
appgurueu pushed a commit to y5nw/minetest that referenced this pull request Mar 4, 2024
appgurueu added a commit to appgurueu/minetest that referenced this pull request Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug One approval ✅ ◻️ Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants