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

Fix buffer overrun with enums pointers cast to int64_t* when enum is only 32-bit #1687

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

bgie
Copy link
Contributor

@bgie bgie commented Jan 20, 2025

This replaces godotengine/godot#101639

Bug description (copied from that issue):
the affected code receives a pointer to an enum as a void pointer, and reinterpret casts it to an int64 pointer, probably because int64 is the size used for variants. But enums are not 64 bit by default, and the majority of enums in the godot codebase use the default size, which is usually int (32 bit on most platforms), and never larger unless absolutely needed, see C++ standard on enum size
Since the code immediately does a truncating C-style cast of this int64 to the enum type, the 4 extra bytes will be discarded immediately and not result in any program logic errors.

I verified my changes by diffing the gen/ directories before and after my changes. As intended, only regular enums are affected, bitfields are already 64 bit and do not require an extra copy.

@bgie bgie requested a review from a team as a code owner January 20, 2025 09:26
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good to me :-)

@dsnopek dsnopek merged commit b86cf32 into godotengine:master Jan 21, 2025
11 checks passed
@dsnopek
Copy link
Collaborator

dsnopek commented Jan 27, 2025

Cherry-picked for 4.2 in PR #1694

@dsnopek
Copy link
Collaborator

dsnopek commented Jan 27, 2025

Cherry-picked for 4.3 in PR #1695

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants