From e7ebda975a4c9f9b8136a571df3c523931358f5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Mon, 8 Jun 2020 11:36:41 +0200 Subject: [PATCH] VariantParser: Fix crash on malformed vectors Each time `r_err_str` is set, we should return a parse error. Removed redundant `return OK;` which were already handled after the big `if`/`else if`/`else` for `TK_IDENTIFIER`. Part of #17372. --- core/variant_parser.cpp | 79 ++++++++--------------------------------- 1 file changed, 15 insertions(+), 64 deletions(-) diff --git a/core/variant_parser.cpp b/core/variant_parser.cpp index bdcad033530..74f4f32c0e4 100644 --- a/core/variant_parser.cpp +++ b/core/variant_parser.cpp @@ -479,12 +479,6 @@ Error VariantParser::_parse_construct(Stream *p_stream, Vector &r_construct, } Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, int &line, String &r_err_str, ResourceParser *p_res_parser) { - /* { - Error err = get_token(p_stream,token,line,r_err_str); - if (err) - return err; - }*/ - if (token.type == TK_CURLY_BRACKET_OPEN) { Dictionary d; Error err = _parse_dictionary(d, p_stream, line, r_err_str, p_res_parser); @@ -501,7 +495,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = a; return OK; - } else if (token.type == TK_IDENTIFIER) { String id = token.value; if (id == "true") { @@ -523,10 +516,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 2) { r_err_str = "Expected 2 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Vector2(args[0], args[1]); - return OK; } else if (id == "Vector2i") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -536,10 +529,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 2) { r_err_str = "Expected 2 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Vector2i(args[0], args[1]); - return OK; } else if (id == "Rect2") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -549,10 +542,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 4) { r_err_str = "Expected 4 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Rect2(args[0], args[1], args[2], args[3]); - return OK; } else if (id == "Rect2i") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -562,10 +555,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 4) { r_err_str = "Expected 4 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Rect2i(args[0], args[1], args[2], args[3]); - return OK; } else if (id == "Vector3") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -575,10 +568,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 3) { r_err_str = "Expected 3 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Vector3(args[0], args[1], args[2]); - return OK; } else if (id == "Vector3i") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -588,12 +581,11 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 3) { r_err_str = "Expected 3 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Vector3i(args[0], args[1], args[2]); - return OK; } else if (id == "Transform2D" || id == "Matrix32") { //compatibility - Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); if (err) { @@ -602,13 +594,14 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 6) { r_err_str = "Expected 6 arguments for constructor"; + return ERR_PARSE_ERROR; } + Transform2D m; m[0] = Vector2(args[0], args[1]); m[1] = Vector2(args[2], args[3]); m[2] = Vector2(args[4], args[5]); value = m; - return OK; } else if (id == "Plane") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -618,10 +611,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 4) { r_err_str = "Expected 4 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Plane(args[0], args[1], args[2], args[3]); - return OK; } else if (id == "Quat") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -631,11 +624,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 4) { r_err_str = "Expected 4 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Quat(args[0], args[1], args[2], args[3]); - return OK; - } else if (id == "AABB" || id == "Rect3") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -645,13 +637,11 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 6) { r_err_str = "Expected 6 arguments for constructor"; + return ERR_PARSE_ERROR; } value = AABB(Vector3(args[0], args[1], args[2]), Vector3(args[3], args[4], args[5])); - return OK; - } else if (id == "Basis" || id == "Matrix3") { //compatibility - Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); if (err) { @@ -660,10 +650,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 9) { r_err_str = "Expected 9 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Basis(args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8]); - return OK; } else if (id == "Transform") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -673,11 +663,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 12) { r_err_str = "Expected 12 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Transform(Basis(args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8]), Vector3(args[9], args[10], args[11])); - return OK; - } else if (id == "Color") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -687,11 +676,10 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, if (args.size() != 4) { r_err_str = "Expected 4 arguments for constructor"; + return ERR_PARSE_ERROR; } value = Color(args[0], args[1], args[2], args[3]); - return OK; - } else if (id == "NodePath") { get_token(p_stream, token, line, r_err_str); if (token.type != TK_PARENTHESIS_OPEN) { @@ -712,7 +700,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, r_err_str = "Expected ')'"; return ERR_PARSE_ERROR; } - } else if (id == "RID") { get_token(p_stream, token, line, r_err_str); if (token.type != TK_PARENTHESIS_OPEN) { @@ -733,8 +720,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, r_err_str = "Expected ')'"; return ERR_PARSE_ERROR; } - - return OK; } else if (id == "Object") { get_token(p_stream, token, line, r_err_str); if (token.type != TK_PARENTHESIS_OPEN) { @@ -834,7 +819,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, at_key = true; } } - } else if (id == "Resource" || id == "SubResource" || id == "ExtResource") { get_token(p_stream, token, line, r_err_str); if (token.type != TK_PARENTHESIS_OPEN) { @@ -850,8 +834,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = res; - - return OK; } else if (p_res_parser && id == "ExtResource" && p_res_parser->ext_func) { RES res; Error err = p_res_parser->ext_func(p_res_parser->userdata, p_stream, res, line, r_err_str); @@ -860,8 +842,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = res; - - return OK; } else if (p_res_parser && id == "SubResource" && p_res_parser->sub_func) { RES res; Error err = p_res_parser->sub_func(p_res_parser->userdata, p_stream, res, line, r_err_str); @@ -870,8 +850,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = res; - - return OK; } else { get_token(p_stream, token, line, r_err_str); if (token.type == TK_STRING) { @@ -889,14 +867,11 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = res; - return OK; - } else { r_err_str = "Expected string as argument for Resource()."; return ERR_PARSE_ERROR; } } - } else if (id == "PackedByteArray" || id == "PoolByteArray" || id == "ByteArray") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -915,9 +890,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; - } else if (id == "PackedInt32Array" || id == "PackedIntArray" || id == "PoolIntArray" || id == "IntArray") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -936,9 +908,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; - } else if (id == "PackedInt64Array") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -957,9 +926,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; - } else if (id == "PackedFloat32Array" || id == "PackedRealArray" || id == "PoolRealArray" || id == "FloatArray") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -978,8 +944,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; } else if (id == "PackedFloat64Array") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -998,8 +962,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; } else if (id == "PackedStringArray" || id == "PoolStringArray" || id == "StringArray") { get_token(p_stream, token, line, r_err_str); if (token.type != TK_PARENTHESIS_OPEN) { @@ -1046,9 +1008,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; - } else if (id == "PackedVector2Array" || id == "PoolVector2Array" || id == "Vector2Array") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -1067,9 +1026,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; - } else if (id == "PackedVector3Array" || id == "PoolVector3Array" || id == "Vector3Array") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -1088,9 +1044,6 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; - } else if (id == "PackedColorArray" || id == "PoolColorArray" || id == "ColorArray") { Vector args; Error err = _parse_construct(p_stream, args, line, r_err_str); @@ -1109,15 +1062,13 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, } value = arr; - - return OK; } else { r_err_str = "Unexpected identifier: '" + id + "'."; return ERR_PARSE_ERROR; } + // All above branches end up here unless they had an early return. return OK; - } else if (token.type == TK_NUMBER) { value = token.value; return OK;