Skip to content

GH-1310 Loosen validation for freed objects in dictionary/array #1311

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.

Sign up for GitHub

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

Open
JoltedJon wants to merge 2 commits into
base: master
Choose a base branch
from
+26 −18
Open

GH-1310 Loosen validation for freed objects in dictionary/array #1311

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73fabca
GH-1310 Loosen validation for freed objects in dictionary
JoltedJon Jul 16, 2026
1d34ed5
GH-1310 Add array as well
JoltedJon Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions core/variant/container_type_validate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
*/

#include "core/object/script_language.h"
#include "core/typedefs.h"
#include "core/variant/variant.h"

struct ContainerType {
Expand All @@ -55,7 +56,7 @@ struct ContainerTypeValidate {

private:
/// Coerces String and StringName into each other and int into float when needed.
_FORCE_INLINE_ bool _internal_validate(Variant &inout_variant, const char *p_operation, bool p_output_errors) const {
_FORCE_INLINE_ bool _internal_validate(Variant &inout_variant, const char *p_operation, bool p_output_errors, bool p_allow_freed = false) const {
if (type == Variant::NIL) {
return true;
}
Expand Down Expand Up @@ -86,10 +87,10 @@ struct ContainerTypeValidate {
return true;
}

return _internal_validate_object(inout_variant, p_operation, p_output_errors);
return _internal_validate_object(inout_variant, p_operation, p_output_errors, p_allow_freed);
}

_FORCE_INLINE_ bool _internal_validate_object(const Variant &p_variant, const char *p_operation, bool p_output_errors) const {
_FORCE_INLINE_ bool _internal_validate_object(const Variant &p_variant, const char *p_operation, bool p_output_errors, bool p_allow_freed = false) const {
ERR_FAIL_COND_V(p_variant.get_type() != Variant::OBJECT, false);

#ifdef DEBUG_ENABLED
Expand All @@ -99,6 +100,9 @@ struct ContainerTypeValidate {
}
Object *object = ObjectDB::get_instance(object_id);
if (object == nullptr) {
if (p_allow_freed) {
return true;
}
if (p_output_errors) {
ERR_FAIL_V_MSG(false, vformat("Attempted to %s an invalid (previously freed?) object instance into a '%s'.", String(p_operation), String(where)));
} else {
Expand Down Expand Up @@ -154,6 +158,10 @@ struct ContainerTypeValidate {
return _internal_validate(inout_variant, p_operation, true);
}

_FORCE_INLINE_ bool validate_for_lookup(Variant &inout_variant, const char *p_operation = "use") const {
return _internal_validate(inout_variant, p_operation, true, true);
}

_FORCE_INLINE_ bool validate_object(const Variant &p_variant, const char *p_operation = "use") const {
return _internal_validate_object(p_variant, p_operation, true);
}
Expand Down
16 changes: 8 additions & 8 deletions core/variant/dictionary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ Variant &Dictionary::operator[](const Variant &p_key) {

const Variant &Dictionary::operator[](const Variant &p_key) const {
Variant key = p_key;
if (unlikely(!_p->typed_key.validate(key, "use `operator[]`"))) {
if (unlikely(!_p->typed_key.validate_for_lookup(key, "use `operator[]`"))) {
if (unlikely(!_p->typed_fallback)) {
_p->typed_fallback = memnew(Variant);
}
Expand All @@ -142,7 +142,7 @@ const Variant &Dictionary::operator[](const Variant &p_key) const {

const Variant *Dictionary::getptr(const Variant &p_key) const {
Variant key = p_key;
if (unlikely(!_p->typed_key.validate(key, "getptr"))) {
if (unlikely(!_p->typed_key.validate_for_lookup(key, "getptr"))) {
return nullptr;
}
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(key));
Expand Down Expand Up @@ -172,7 +172,7 @@ Variant *Dictionary::getptr(const Variant &p_key) {

Variant Dictionary::get_valid(const Variant &p_key) const {
Variant key = p_key;
ERR_FAIL_COND_V(!_p->typed_key.validate(key, "get_valid"), Variant());
ERR_FAIL_COND_V(!_p->typed_key.validate_for_lookup(key, "get_valid"), Variant());
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(key));

if (!E) {
Expand All @@ -183,7 +183,7 @@ Variant Dictionary::get_valid(const Variant &p_key) const {

Variant Dictionary::get(const Variant &p_key, const Variant &p_default) const {
Variant key = p_key;
ERR_FAIL_COND_V(!_p->typed_key.validate(key, "get"), p_default);
ERR_FAIL_COND_V(!_p->typed_key.validate_for_lookup(key, "get"), p_default);
const Variant *result = getptr(key);
if (!result) {
return p_default;
Expand Down Expand Up @@ -225,14 +225,14 @@ bool Dictionary::is_empty() const {

bool Dictionary::has(const Variant &p_key) const {
Variant key = p_key;
ERR_FAIL_COND_V(!_p->typed_key.validate(key, "use 'has'"), false);
ERR_FAIL_COND_V(!_p->typed_key.validate_for_lookup(key, "use 'has'"), false);
return _p->variant_map.has(key);
}

bool Dictionary::has_all(const Array &p_keys) const {
for (int i = 0; i < p_keys.size(); i++) {
Variant key = p_keys[i];
ERR_FAIL_COND_V(!_p->typed_key.validate(key, "use 'has_all'"), false);
ERR_FAIL_COND_V(!_p->typed_key.validate_for_lookup(key, "use 'has_all'"), false);
if (!_p->variant_map.has(key)) {
return false;
}
Expand All @@ -253,7 +253,7 @@ Variant Dictionary::find_key(const Variant &p_value) const {

bool Dictionary::erase(const Variant &p_key) {
Variant key = p_key;
ERR_FAIL_COND_V(!_p->typed_key.validate(key, "erase"), false);
ERR_FAIL_COND_V(!_p->typed_key.validate_for_lookup(key), false);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
ERR_FAIL_COND_V_MSG(_p->read_only, false, "Dictionary is in read-only state.");
return _p->variant_map.erase(key);
}
Expand Down Expand Up @@ -572,7 +572,7 @@ const Variant *Dictionary::next(const Variant *p_key) const {
return nullptr;
}
Variant key = *p_key;
ERR_FAIL_COND_V(!_p->typed_key.validate(key, "next"), nullptr);
ERR_FAIL_COND_V(!_p->typed_key.validate_for_lookup(key, "next"), nullptr);
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::Iterator E = _p->variant_map.find(key);

if (!E) {
Expand Down
Loading