Fix a crash & add ftest to cover other reported cases

CHANGES_NEXT_RELEASE

@@ -1,6 +1,8 @@
 - Add: TextUnrestricted attribute type to avoid forbidden chars checking (#3550)
 - Add: notification flow control on update (#3568)
 - Fix: crash when using null character in some NGSIv1 operations
+- Fix: crash when using attributes with invalid JSON having more than one "value" keys (#3603)
+- Hardening: add deepness control to compound attribute and metadata value (max 50 levels) (related with #3605, #3606 and #3608)
 - Hardening: NULL control in some strdup and calloc cases (very unlikely, but theoretically possible) (#3578)
 - Hardening: avoid crash in the unlikely case MongoDB get databases operations fails in csubs cache refresh logic (#3456, partly)
 - Hardening: refactor mongo connection logic at startup to make it simpler

doc/manuals/user/known_limitations.md

@@ -61,3 +61,19 @@ As a reference, in our lab tests in a machine with Orion 1.13.0 running with 4 G
 of subscriptions got higher than 211.000 subscriptions.
 
 There is [an issue in the repository](https://github.com/telefonicaid/fiware-orion/issues/2780) about improvements related with this.
+
+## Maximum nesting level
+
+There is a maximum nesting level for compound attributes and metadata values. This limit is 50 levels.
+If you try to create or update an attribute/metadata with a value overpassing this limit you will get a
+400 Bad Request error with this payload:
+
+```
+{
+   "description": "attribute or metadata value has overpassed maximum nesting limit: 50",
+   "error": "ParseError"
+}
+```
+
+Note that other systems dealing with JSON also use to have this kind of limit (for instance,
+[the limit in MongoDB](https://docs.mongodb.com/manual/reference/limits/#Nested-Depth-for-BSON-Documents)).

src/lib/common/errorMessages.h

@@ -48,6 +48,7 @@
 
 #define ERROR_PARSE                                   "ParseError"
 #define ERROR_DESC_PARSE                              "Errors found in incoming JSON buffer"
+#define ERROR_DESC_PARSE_MAX_JSON_NESTING             "attribute or metadata value has overpassed maximum nesting limit: " STR(MAX_JSON_NESTING)
 
 #define ERROR_BAD_REQUEST                             "BadRequest"
 #define ERROR_DESC_BAD_REQUEST_INVALID_CHAR_URI       "invalid character in URI"

src/lib/common/limits.h

@@ -209,4 +209,15 @@
 
 
 
+/* ****************************************************************************
+*
+* MAX_JSON_NESTING -
+*
+* 50 seems to be a reasonable limit (note that MongoDB sets this limit in 100
+* see https://docs.mongodb.com/manual/reference/limits/#Nested-Depth-for-BSON-Documents
+*/
+#define MAX_JSON_NESTING  50
+
+
+
 #endif  // SRC_LIB_COMMON_LIMITS_H_

src/lib/jsonParse/jsonParse.cpp

@@ -24,6 +24,8 @@
 */
 #include <stdint.h>
 
+#include "common/limits.h"
+
 //
 // http://www.boost.org/doc/libs/1_31_0/libs/spirit/doc/grammar.html:
 //
@@ -251,15 +253,29 @@ std::string nodeType(const std::string& nodeName, const std::string& value, orio
 /* ****************************************************************************
 *
 * eatCompound -
+*
+* This is a recusive function.
 */
-void eatCompound
+static void eatCompound
 (
   ConnectionInfo*                           ciP,
   orion::CompoundValueNode*                 containerP,
   boost::property_tree::ptree::value_type&  v,
-  const std::string&                        indent
+  const std::string&                        indent,
+  int                                       deep
 )
 {
+  if (deep > MAX_JSON_NESTING)
+  {
+    std::string details = std::string("compound attribute value has overpassed maximum nesting limit");
+    alarmMgr.badInput(clientIp, details);
+
+    ciP->httpStatusCode = SccBadRequest;
+    ciP->answer = details;
+
+    return;
+  }
+
   std::string                  nodeName     = v.first.data();
   std::string                  nodeValue    = v.second.data();
   boost::property_tree::ptree  subtree1     = (boost::property_tree::ptree) v.second;
@@ -327,7 +343,7 @@ void eatCompound
   boost::property_tree::ptree subtree = (boost::property_tree::ptree) v.second;
   BOOST_FOREACH(boost::property_tree::ptree::value_type &v2, subtree)
   {
-    eatCompound(ciP, containerP, v2, indent + "  ");
+    eatCompound(ciP, containerP, v2, indent + "  ", deep + 1);
   }
 }
 
@@ -382,7 +398,7 @@ static std::string jsonParse
   {
 
     LM_T(LmtCompoundValue, ("Calling eatCompound for '%s'", path.c_str()));
-    eatCompound(ciP, NULL, v, "");
+    eatCompound(ciP, NULL, v, "", 0);
     compoundValueEnd(ciP, parseDataP);
 
     if (ciP->httpStatusCode != SccOk)

src/lib/jsonParseV2/parseAttributeValue.cpp

@@ -66,7 +66,22 @@ std::string parseAttributeValue(ConnectionInfo* ciP, ContextAttribute* caP)
   }
 
   caP->valueType  = (document.IsObject())? orion::ValueTypeObject : orion::ValueTypeVector;
-  parseContextAttributeCompoundValueStandAlone(document, caP);
+  std::string r   = parseContextAttributeCompoundValueStandAlone(document, caP);
+
+  if (r == "max deep reached")
+  {
+    OrionError oe(SccBadRequest, ERROR_DESC_PARSE_MAX_JSON_NESTING, ERROR_PARSE);
+    alarmMgr.badInput(clientIp, r);
+    ciP->httpStatusCode = SccBadRequest;
+    return oe.toJson();
+  }
+  else if (r != "OK")  // other error cases get a general treatment
+  {
+    OrionError oe(SccBadRequest, r, "BadRequest");
+    alarmMgr.badInput(clientIp, r);
+    ciP->httpStatusCode = SccBadRequest;
+    return oe.toJson();
+  }
 
   return "OK";
 }

src/lib/jsonParseV2/parseContextAttribute.cpp

@@ -58,6 +58,11 @@ static std::string parseContextAttributeObject
   // valueTypeNotGiven will be overridden inside the 'for' block in case the attribute has an actual value
   caP->valueType = orion::ValueTypeNull;
 
+  // It may happen in the for iterator to see the same key twice. This is a problem for "value" in the
+  // case of compounds and may lead to a crash (see details in issue #3603). Thus, we need to control
+  // explicitely value has been set with this flag
+  bool valueSet = false;
+
   for (rapidjson::Value::ConstMemberIterator iter = start.MemberBegin(); iter != start.MemberEnd(); ++iter)
   {
     std::string name   = iter->name.GetString();
@@ -67,7 +72,6 @@ static std::string parseContextAttributeObject
     {
       if (type != "String")
       {
-        alarmMgr.badInput(clientIp, "ContextAttributeObject::type must be a String");
         return "invalid JSON type for attribute type";
       }
 
@@ -76,6 +80,12 @@ static std::string parseContextAttributeObject
     }
     else if (name == "value")
     {
+      if (valueSet)
+      {
+        return "duplicated value key in attribute";
+      }
+      valueSet = true;
+
       if (type == "String")
       {
         caP->stringValue  = iter->value.GetString();
@@ -116,22 +126,19 @@ static std::string parseContextAttributeObject
         //
         caP->valueType  = orion::ValueTypeVector;
         *compoundVector = true;
-        std::string r   = parseContextAttributeCompoundValue(iter, caP, NULL);
+        std::string r   = parseContextAttributeCompoundValue(iter, caP, NULL, 0);
         if (r != "OK")
         {
-          alarmMgr.badInput(clientIp, "json error in ContextAttributeObject::Vector");
-          return "json error in ContextAttributeObject::Vector";
+          return r;
         }
       }
       else if (type == "Object")
       {
-        caP->valueType    = orion::ValueTypeObject;
-
-        std::string r = parseContextAttributeCompoundValue(iter, caP, NULL);
+        caP->valueType  = orion::ValueTypeObject;
+        std::string r   = parseContextAttributeCompoundValue(iter, caP, NULL, 0);
         if (r != "OK")
         {
-          alarmMgr.badInput(clientIp, "json error in ContextAttributeObject::Object");
-          return "json error in ContextAttributeObject::Object";
+          return r;
         }
       }
     }
@@ -141,8 +148,6 @@ static std::string parseContextAttributeObject
 
       if (r != "OK")
       {
-        std::string details = std::string("error parsing Metadata: ") + r;
-        alarmMgr.badInput(clientIp, details);
         return r;
       }
     }
@@ -226,9 +231,15 @@ std::string parseContextAttribute
     {
       compoundVector  = true;
       caP->valueType  = orion::ValueTypeObject;
-      std::string r   = parseContextAttributeCompoundValue(iter, caP, NULL);
+      std::string r   = parseContextAttributeCompoundValue(iter, caP, NULL, 0);
 
-      if (r != "OK")
+      if (r == "max deep reached")
+      {
+        alarmMgr.badInput(clientIp, "max deep reached in ContextAttributeObject::Vector");
+        ciP->httpStatusCode = SccBadRequest;
+        return "max deep reached";
+      }
+      else if (r != "OK")  // other error cases get a general treatment
       {
         alarmMgr.badInput(clientIp, "json error in ContextAttribute::Vector");
         ciP->httpStatusCode = SccBadRequest;
@@ -237,8 +248,21 @@ std::string parseContextAttribute
     }
     else if (type == "Object")
     {
-      parseContextAttributeCompoundValue(iter, caP, NULL);
       caP->valueType = orion::ValueTypeObject;
+      std::string r = parseContextAttributeCompoundValue(iter, caP, NULL, 0);
+
+      if (r == "max deep reached")
+      {
+        alarmMgr.badInput(clientIp, "max deep reached in ContextAttributeObject::Object");
+        ciP->httpStatusCode = SccBadRequest;
+        return "max deep reached";
+      }
+      else if (r != "OK")  // other error cases get a general treatment
+      {
+        alarmMgr.badInput(clientIp, "json error in ContextAttribute::Object");
+        ciP->httpStatusCode = SccBadRequest;
+        return "json error in ContextAttribute::Object";
+      }
     }
     else
     {
@@ -263,7 +287,13 @@ std::string parseContextAttribute
     if (iter->value.HasMember("value") || ciP->apiVersion == V2)
     {
       std::string r = parseContextAttributeObject(iter->value, caP, &compoundVector);
-      if (r != "OK")
+      if (r == "max deep reached")
+      {
+        alarmMgr.badInput(clientIp, "max deep reached in ContextAttributeObject::Object");
+        ciP->httpStatusCode = SccBadRequest;
+        return "max deep reached";
+      }
+      else if (r != "OK")  // other error cases get a general treatment
       {
         alarmMgr.badInput(clientIp, "JSON parse error in ContextAttribute::Object");
         ciP->httpStatusCode = SccBadRequest;
@@ -330,7 +360,16 @@ std::string parseContextAttribute(ConnectionInfo* ciP, ContextAttribute* caP)
   bool         compoundVector = false;
   std::string  r = parseContextAttributeObject(document, caP, &compoundVector);
 
-  if (r != "OK")
+  if (r == "max deep reached")
+  {
+    OrionError oe(SccBadRequest, ERROR_DESC_PARSE_MAX_JSON_NESTING, ERROR_PARSE);
+
+    alarmMgr.badInput(clientIp, r);
+    ciP->httpStatusCode = SccBadRequest;
+
+    return oe.toJson();
+  }
+  else if (r != "OK")  // other error cases get a general treatment
   {
     OrionError oe(SccBadRequest, r, "BadRequest");