From ccbd810934a6c8667ca71ef788d084a4ac37a6e0 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Fri, 30 Jul 2021 15:43:01 +0200 Subject: [PATCH] [Net] Fix Marshalls infinite recursion crash. Variants like dictionaries and arrays can have cyclic references, which caused `encode_variant` to run an infinite recursion. Instead of keeping a stack and looking for cyclic references which would make serialization slower, this commit adds a `MAX_RECURSION_DEPTH` constant to Variant, and have `encode_variant` keep track of the current recursion depth, bailing when it's too high since this likely means a cyclic reference has been encountered. (cherry picked from commit 324636473aa65165caeee29e9b70e2d8c21fcb96) --- core/io/marshalls.cpp | 18 ++++++++++-------- core/io/marshalls.h | 2 +- core/variant.h | 5 +++++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/core/io/marshalls.cpp b/core/io/marshalls.cpp index 59fea4fba65..7c6bd718694 100644 --- a/core/io/marshalls.cpp +++ b/core/io/marshalls.cpp @@ -763,7 +763,8 @@ static void _encode_string(const String &p_string, uint8_t *&buf, int &r_len) { } } -Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bool p_full_objects) { +Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bool p_full_objects, int p_depth) { + ERR_FAIL_COND_V_MSG(p_depth > Variant::MAX_RECURSION_DEPTH, ERR_OUT_OF_MEMORY, "Potential inifite recursion detected. Bailing."); uint8_t *buf = r_buffer; r_len = 0; @@ -1076,10 +1077,8 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo _encode_string(E->get().name, buf, r_len); int len; - Error err = encode_variant(obj->get(E->get().name), buf, len, p_full_objects); - if (err) { - return err; - } + Error err = encode_variant(obj->get(E->get().name), buf, len, p_full_objects, p_depth + 1); + ERR_FAIL_COND_V(err, err); ERR_FAIL_COND_V(len % 4, ERR_BUG); r_len += len; if (buf) { @@ -1130,13 +1129,15 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo */ Variant *v = d.getptr(E->get()); int len; - encode_variant(v ? E->get() : Variant("[Deleted Object]"), buf, len, p_full_objects); + Error err = encode_variant(v ? E->get() : Variant("[Deleted Object]"), buf, len, p_full_objects, p_depth + 1); + ERR_FAIL_COND_V(err, err); ERR_FAIL_COND_V(len % 4, ERR_BUG); r_len += len; if (buf) { buf += len; } - encode_variant(v ? *v : Variant(), buf, len, p_full_objects); + err = encode_variant(v ? *v : Variant(), buf, len, p_full_objects, p_depth + 1); + ERR_FAIL_COND_V(err, err); ERR_FAIL_COND_V(len % 4, ERR_BUG); r_len += len; if (buf) { @@ -1157,7 +1158,8 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo for (int i = 0; i < v.size(); i++) { int len; - encode_variant(v.get(i), buf, len, p_full_objects); + Error err = encode_variant(v.get(i), buf, len, p_full_objects, p_depth + 1); + ERR_FAIL_COND_V(err, err); ERR_FAIL_COND_V(len % 4, ERR_BUG); r_len += len; if (buf) { diff --git a/core/io/marshalls.h b/core/io/marshalls.h index f7935729bf7..c52616affdb 100644 --- a/core/io/marshalls.h +++ b/core/io/marshalls.h @@ -181,6 +181,6 @@ public: }; Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int *r_len = nullptr, bool p_allow_objects = false); -Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bool p_full_objects = false); +Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bool p_full_objects = false, int p_depth = 0); #endif diff --git a/core/variant.h b/core/variant.h index 8db69886b07..b53288733db 100644 --- a/core/variant.h +++ b/core/variant.h @@ -136,6 +136,11 @@ public: }; + enum { + // Maximum recursion depth allowed when serializing variants. + MAX_RECURSION_DEPTH = 1024, + }; + private: friend struct _VariantCall; // Variant takes 20 bytes when real_t is float, and 36 if double