Merge pull request #1531 from FIWARE/hardening/valgrind

Better error handling of empty expiresAt, actually, avoiding an error detected by valgrind
FIWARE · Jan 12, 2024 · 5165cf1 · 5165cf1
2 parents 997e3e5 + a3481cd
commit 5165cf1
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 9 deletions.
diff --git a/src/lib/orionld/common/dateTime.cpp b/src/lib/orionld/common/dateTime.cpp
@@ -435,6 +435,12 @@ double dateTimeFromString(const char* iso8601String, char* errorString, int erro
 {
   char iso8601[64];
 
+  if (iso8601String == NULL)
+    LM_RE(-1, ("NULL ISO8601 String"));
+
+  if (*iso8601String == 0)
+    LM_RE(-1, ("Empty ISO8601 String"));
+
   strncpy(iso8601, iso8601String, sizeof(iso8601) - 1);
 
   char* date = stringStrip(iso8601);

diff --git a/src/lib/orionld/common/stringStrip.cpp b/src/lib/orionld/common/stringStrip.cpp
@@ -23,6 +23,7 @@
 * Author: Ken Zangelin
 */
 #include <string.h>                                            // strlen
+#include <unistd.h>                                            // NULL
 
 #include "orionld/common/stringStrip.h"                        // Own interface
 
@@ -34,10 +35,17 @@
 //
 char* stringStrip(char* s)
 {
+  if (s == NULL)
+    return NULL;
+
   // Remove all whitespace before
   while ((*s == ' ') || (*s == '\t') || (*s == '\n'))
     ++s;
 
+  // Nothing left?
+  if (*s == 0)
+    return (char*) "";
+
   // Remove all whitespace after
   char* sEnd = &s[strlen(s) - 1];
   while ((*sEnd == ' ') || (*sEnd == '\t') || (*sEnd == '\n'))

diff --git a/src/lib/orionld/payloadCheck/pCheckSubscription.cpp b/src/lib/orionld/payloadCheck/pCheckSubscription.cpp
@@ -272,6 +272,7 @@ bool pCheckSubscription
     else if ((strcmp(subItemP->name, "expiresAt") == 0) || (strcmp(subItemP->name, "expires") == 0))
     {
       PCHECK_STRING(subItemP, 0, NULL, SubscriptionExpiresAtPath, 400);
+      PCHECK_STRING_EMPTY(subItemP, 0, NULL, SubscriptionExpiresAtPath, 400);
       PCHECK_DUPLICATE(expiresAtP, subItemP, 0, NULL, SubscriptionExpiresAtPath, 400);
       PCHECK_ISO8601(expiresAt, expiresAtP->value.s, 0, NULL, SubscriptionExpiresAtPath, 400);
       PCHECK_EXPIRESAT_IN_FUTURE(0, "Invalid Subscription", "/expiresAt/ in the past", 400, expiresAt, orionldState.requestTime);

diff --git a/src/lib/orionld/types/OrionldMimeType.cpp b/src/lib/orionld/types/OrionldMimeType.cpp
@@ -41,6 +41,9 @@ MimeType mimeTypeFromString(const char* mimeType, char** charsetP, bool wildcard
   char* s;
   char* cP = (char*) mimeType;
 
+  if ((cP == NULL) || (*cP == 0))
+    return MT_NONE;
+
   if ((s = strstr(cP, ";")) != NULL)
   {
     *s = 0;

diff --git a/src/lib/rest/StringFilter.cpp b/src/lib/rest/StringFilter.cpp
@@ -208,7 +208,7 @@ bool StringFilterItem::rangeParse(char* s, std::string* errorStringP)
   toString   = stringStrip(toString);
   fromString = stringStrip(fromString);
 
-  if ((*toString == 0) || (*fromString == 0))
+  if ((toString == NULL) || (*toString == 0) || (fromString == NULL) || (*fromString == 0))
   {
     *errorStringP = "empty item in range";
     return false;
@@ -278,7 +278,7 @@ bool StringFilterItem::listItemAdd(char* s, std::string* errorStringP)
   bool                   b;
 
   s = stringStrip(s);
-  if (*s == 0)
+  if ((s == NULL) || (*s == 0))
   {
     *errorStringP = "empty item in list";
     return false;
@@ -357,6 +357,8 @@ bool StringFilterItem::listParse(char* s, std::string* errorStringP)
   char* cP        = itemStart;
   bool  inString  = false;
 
+  if (cP == NULL) cP = (char*) "";
+
   while (*cP != 0)
   {
     if (*cP == '\'')
@@ -589,7 +591,7 @@ bool StringFilterItem::parse(char* qItem, std::string* errorStringP, StringFilte
   type = _type;
 
   s = stringStrip(s);
-  if (*s == 0)
+  if ((s == NULL) || (*s == 0))
   {
     free(toFree);
     *errorStringP = "empty q-item";
@@ -622,7 +624,7 @@ bool StringFilterItem::parse(char* qItem, std::string* errorStringP, StringFilte
   // Check for invalid LHS
   // LHS can be empty ONLY if UNARY OP, i.e. SfopNotExists OR SfopExists
   //
-  if ((*lhs == 0) && (op != SfopNotExists) && (op != SfopExists))
+  if (((lhs == NULL) || (*lhs == 0)) && (op != SfopNotExists) && (op != SfopExists))
   {
     *errorStringP = "empty left-hand-side in q-item";
     free(toFree);
@@ -688,7 +690,7 @@ bool StringFilterItem::parse(char* qItem, std::string* errorStringP, StringFilte
   //
   // Check for empty RHS
   //
-  if (*rhs == 0)
+  if ((rhs == NULL) || (*rhs == 0))
   {
     *errorStringP = "empty right-hand-side in q-item";
     free(toFree);

diff --git a/test/functionalTest/cases/0000_ngsild/ngsild_subscription_patch_errors.test b/test/functionalTest/cases/0000_ngsild/ngsild_subscription_patch_errors.test
@@ -2178,14 +2178,13 @@ Date: REGEX(.*)
 89. Attempt to PATCH the subscription with an 'expires' that is an empty string
 ===============================================================================
 HTTP/1.1 400 Bad Request
-Content-Length: 179
+Content-Length: 119
 Content-Type: application/json
 Date: REGEX(.*)
 
 {
-    "detail": "invalid DateTime: '' (error in 'date' part '')",
-    "field": "Subscription::expiresAt",
-    "title": "Invalid ISO8601",
+    "detail": "Subscription::expiresAt",
+    "title": "Empty String",
     "type": "https://uri.etsi.org/ngsi-ld/errors/BadRequestData"
 }