Bug#23707238 - PROTOBUF LIMITS THE NUMBER OF NESTED OBJECTS TO 100 RECURSIONS

Description:
    When X Plugin receives X Protocol message that contains 101 nested objects,
    the decoding operation fails and client received following error:
    "Invalid message" and gets disconnected.

Analysis:
    Decoding failure is caused by Protobuf which limits the number of nested objects.
    There isn't any way to verify the cause of the failure, which leaves us with
    a generic message. When decoding this message manualy (by "protoc") it doesn't
    give any usefull information about the cause.

Fix:
    Fix sets the limit to 100 (in explicit to ensure that we do not overwrite the stack)
    It is done by calling following method:
    google::protobuf::io::CodedInputStream::SetRecursionLimit
    Used a workaround for checking the error. When decoding failed, plugin call
    DecrementRecursionDepth and IncrementRecursionDepth on CodedInputStream,
    if the increment fails this mean that the limit was reached and correspondin error information is
    send back.

RB: 13186
Reviewed-by: Grzegorz Szwarc <[email protected]>
Reviewed-by: Andrzej Religa <[email protected]>

Author: lkotula

rapid/plugin/x/mysqlxtest_src/mysqlxtest.cc

@@ -822,8 +822,8 @@ class Command
     m_commands["enablessl"]   = &Command::cmd_enablessl;
     m_commands["sleep "]      = &Command::cmd_sleep;
     m_commands["login "]      = &Command::cmd_login;
-    m_commands["stmtadmin "]  = &Command::cmd_stmt_admin;
-    m_commands["stmtsql "]    = &Command::cmd_stmt_sql;
+    m_commands["stmtadmin "]  = &Command::cmd_stmtadmin;
+    m_commands["stmtsql "]    = &Command::cmd_stmtsql;
     m_commands["loginerror "] = &Command::cmd_loginerror;
     m_commands["repeat "]     = &Command::cmd_repeat;
     m_commands["endrepeat"]   = &Command::cmd_endrepeat;
@@ -1149,7 +1149,7 @@ class Command
     return Continue;
   }
 
-  Result cmd_stmt_sql(Execution_context &context, const std::string &args)
+  Result cmd_stmtsql(Execution_context &context, const std::string &args)
   {
     Mysqlx::Sql::StmtExecute stmt;
 
@@ -1165,7 +1165,7 @@ class Command
   }
 
 
-  Result cmd_stmt_admin(Execution_context &context, const std::string &args)
+  Result cmd_stmtadmin(Execution_context &context, const std::string &args)
   {
     std::string tmp = args;
     replace_variables(tmp);
@@ -1296,6 +1296,9 @@ class Command
       variable_name = argl[1];
     }
 
+    // Allow use of variables as a source of number of iterations
+    replace_variables(argl[0]);
+
     Loop_do loop = {context.m_stream.tellg(), atoi(argl[0].c_str()), 0, variable_name};
 
     m_loop_stack.push_back(loop);

rapid/plugin/x/ngs/src/protocol_decoder.cc

@@ -98,6 +98,7 @@ Message *Message_decoder::alloc_message(int8_t type, Error_code &ret_error, bool
 
 Error_code Message_decoder::parse(Request &request)
 {
+  const int max_recursion_limit = 100;
   bool msg_is_shared;
   Error_code ret_error;
   Message *message = alloc_message(request.get_type(), ret_error, msg_is_shared);
@@ -109,12 +110,29 @@ Error_code Message_decoder::parse(Request &request)
                                                   static_cast<int>(buffer.length()));
     // variable 'mysqlx_max_allowed_packet' has been checked when buffer was filling by data
     stream.SetTotalBytesLimit(static_cast<int>(buffer.length()), -1 /*no warnings*/);
+    // Protobuf limits the number of nested objects when decoding messages
+    // lets set the value in explicit way (to ensure that is set accordingly with
+    // out stack size)
+    //
+    // Protobuf doesn't print a readable error after reaching the limit
+    // thus in case of failure we try to validate the limit by decrementing and
+    // incrementing the value & checking result for failure
+    stream.SetRecursionLimit(max_recursion_limit);
+
     message->ParseFromCodedStream(&stream);
 
     if (!message->IsInitialized())
     {
       log_debug("Error parsing message of type %i: %s",
                 request.get_type(), message->InitializationErrorString().c_str());
+
+      //Workaraound
+      stream.DecrementRecursionDepth();
+      if (!stream.IncrementRecursionDepth())
+      {
+        return Error(ER_X_BAD_MESSAGE, "X Protocol message recursion limit (%i) exceeded", max_recursion_limit);
+      }
+
       if (!msg_is_shared)
         delete message;
       message = NULL;

rapid/plugin/x/protocol/mysqlx_datatypes.proto

@@ -31,14 +31,14 @@ message Scalar {
     required bytes value = 1;
     optional uint64 collation = 2;
   };
- 
-// an opaque octet sequence, with an optional content_type
-// See ``Mysqlx.Resultset.ColumnMetadata`` for list of known values.
-message Octets {
-  required bytes value = 1;
-  optional uint32 content_type = 2;
-};
-  
+
+  // an opaque octet sequence, with an optional content_type
+  // See ``Mysqlx.Resultset.ColumnMetadata`` for list of known values.
+  message Octets {
+    required bytes value = 1;
+    optional uint32 content_type = 2;
+  };
+
   enum Type {
     V_SINT = 1;
     V_UINT = 2;

rapid/plugin/x/tests/mtr/r/protobuf_nested.result

@@ -0,0 +1,23 @@
+install plugin mysqlx soname "mysqlx.so";
+call mtr.add_suppression("Plugin mysqlx reported: .Failed at SSL configuration: .SSL context is not usable without certificate and private key..");
+call mtr.add_suppression("Plugin mysqlx reported: .SSL_CTX_load_verify_locations failed.");
+
+command ok
+connecting...
+active session is now 'test_different_messages'
+Try to use value greater than the default limit 50 /2 => 25
+doc
+command ok
+Try to use value equal than the X Protocol limit 100 /2 => 50
+doc
+command ok
+Try to use value greater than the X Protocol limit 102 /2 => 51
+Got expected error: X Protocol message recursion limit (100) exceeded (code 5000)
+aborting session test_different_messages
+switched to session default
+Mysqlx.Ok {
+  msg: "bye!"
+}
+ok
+uninstall plugin mysqlx;
+DROP TABLE IF EXISTS coll;

rapid/plugin/x/tests/mtr/t/protobuf_nested-master.opt

@@ -0,0 +1 @@
+--plugin_dir=$MYSQLXPLUGIN_DIR

rapid/plugin/x/tests/mtr/t/protobuf_nested.test

@@ -0,0 +1,98 @@
+ ## Preamble
+--source ../include/xplugin_preamble.inc
+
+## TEST STARTS HERE
+--write_file $MYSQL_TMP_DIR/mysqlx-update_itemremove.tmp
+-->stmtadmin create_collection	 {"schema":"test", "name":"coll"}
+-->recvresult
+
+## It tests the limits of protobuf, we do not need any data in the collection
+
+
+-->macro Send_message_with_repeated_nested_objects	%OBJECTS_TO_NEST%
+-->varlet %NESTED_OBJECTS% 
+
+-->varlet %OBJECTS_TO_REPEAT% %OBJECTS_TO_NEST%
+-->varinc %OBJECTS_TO_REPEAT% -3
+
+-->repeat %OBJECTS_TO_REPEAT%	%ITERATION%
+-->varlet %OBJECT_NUMBER% %ITERATION%
+-->varinc %OBJECT_NUMBER% 4
+-->varlet %NESTED_OBJECTS% %NESTED_OBJECTS%  value { type: ARRAY array { value { type: LITERAL literal { type: V_SINT v_signed_int: %OBJECT_NUMBER% } } 
+-->endrepeat
+
+-->repeat %OBJECTS_TO_REPEAT%
+-->varlet %NESTED_OBJECTS% %NESTED_OBJECTS%  } } 
+-->endrepeat
+
+-->quiet
+Mysqlx.Crud.Find {
+  collection {
+    name: "coll"
+    schema: "test"
+  }
+  data_model: DOCUMENT
+  criteria {
+    type: OPERATOR
+    operator {
+      name: "=="
+      param {
+        type: IDENT
+        identifier {
+          document_path {
+            type: MEMBER
+            value: "ARR0"
+          }
+        }
+      }
+      param {
+        type: ARRAY
+        array {
+          value {
+            type: LITERAL
+            literal {
+              type: V_SINT
+              v_signed_int: 3
+            }
+          }
+          
+          %NESTED_OBJECTS%
+          
+          
+        }
+        
+      }
+    }
+  }
+}
+-->noquiet
+-->endmacro
+
+## Current recursion limit is set to 100
+## each repeated message in bellow macro contains two messages
+## (it increases the current depth by 2)
+-->newsession test_different_messages	root
+-->echo Try to use value greater than the default limit 50 /2 => 25
+-->callmacro Send_message_with_repeated_nested_objects	25
+-->recvresult
+
+-->echo Try to use value equal than the X Protocol limit 100 /2 => 50
+-->callmacro Send_message_with_repeated_nested_objects	50
+-->recvresult
+
+-->echo Try to use value greater than the X Protocol limit 102 /2 => 51
+-->callmacro Send_message_with_repeated_nested_objects	51
+-->expecterror 5000
+-->recvresult
+-->closesession abort
+
+EOF
+
+--exec $MYSQLXTEST -u root  --file=$MYSQL_TMP_DIR/mysqlx-update_itemremove.tmp 2>&1
+
+## Postamble
+--remove_file $MYSQL_TMP_DIR/mysqlx-update_itemremove.tmp
+
+uninstall plugin mysqlx;
+
+DROP TABLE IF EXISTS coll;