Merge pull request #15902 from ghalliday/safeStrings

HPCC-27375 Try and catch potential use-after free for StringBuffer

Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/hql/hqldsparam.cpp

@@ -75,7 +75,8 @@ IHqlExpression* HqlGram::processAbstractDataset(IHqlExpression* _expr, IHqlExpre
         }
         else if (errorIfNotFound)
         {
-            reportError(ERR_DSPARM_MISSINGFIELD,errpos,"Dataset %s has no field named '%s'", str(actual->queryName()), str(mapto));
+            const char * actualName = str(actual->queryName());
+            reportError(ERR_DSPARM_MISSINGFIELD,errpos,"Dataset %s has no field named '%s'", actualName ? actualName : "<unnamed>", str(mapto));
             hadError = true;
         }
     }       

system/jlib/jstring.cpp

@@ -100,17 +100,21 @@ StringBuffer::StringBuffer(StringBuffer && value)
     swapWith(value);
 }
 
-StringBuffer::StringBuffer(bool useInternal)
-{
-    if (useInternal)
-        init();
-    else
-        initNoInternal();
-}
-
 StringBuffer::~StringBuffer()
 {
+#ifdef CATCH_USE_AFTER_FREE
+    if (buffer == internalBuffer)
+        strncpy(internalBuffer, "use-after-free", InternalBufferSize-1);
+    else
+        strncpy(buffer, "use-after-free", curLen);
+#endif
     freeBuffer();
+#ifdef CATCH_USE_AFTER_FREE
+    //Try and cause any use after free to access invalid memory
+    curLen = 0;
+    maxLen = (size_t)-1;
+    buffer = nullptr;
+#endif
 }
 
 void StringBuffer::freeBuffer()
@@ -1102,7 +1106,7 @@ VStringBuffer::VStringBuffer(const char* format, ...)
 
 //===========================================================================
 
-StringAttrBuilder::StringAttrBuilder(StringAttr & _target) : StringBuffer(false), target(_target)
+StringAttrBuilder::StringAttrBuilder(StringAttr & _target) : target(_target)
 {
 }
 
@@ -1146,7 +1150,11 @@ String::String(StringBuffer & value)
 
 String::~String()
 {
-  if (text != TheNullStr) free(text);
+    if (text != TheNullStr)
+        free(text);
+#ifdef CATCH_USE_AFTER_FREE
+    text = nullptr;
+#endif
 }
 
 char String::charAt(size32_t index) const

system/jlib/jstring.hpp

@@ -26,6 +26,9 @@
 #include "jbuff.hpp"
 
 // A Java compatible String and StringBuffer class - useful for dynamic strings.
+#ifdef _DEBUG
+#define CATCH_USE_AFTER_FREE
+#endif
 
 class String;
 interface IAtom;
@@ -41,7 +44,6 @@ public:
     explicit StringBuffer(StringBuffer && value);
     explicit StringBuffer(size_t len, const char *value);
     explicit StringBuffer(const StringBuffer & value);
-    explicit StringBuffer(bool useInternal);
     explicit StringBuffer(char value);
     ~StringBuffer();
 
@@ -153,12 +155,6 @@ protected:
         curLen = 0;
         maxLen = InternalBufferSize;
     }
-    void initNoInternal()
-    {
-        buffer = NULL;
-        curLen = 0;
-        maxLen = 0;
-    }
     void freeBuffer();
     void _insert(size_t offset, size_t insertLen);
     void _realloc(size_t newLen);
@@ -259,7 +255,13 @@ public:
     StringAttr(StringAttr && src);
     StringAttr& operator = (StringAttr && from);
     StringAttr& operator = (const StringAttr & from);
-    inline ~StringAttr(void) { free(text); }
+    inline ~StringAttr(void)
+    {
+        free(text);
+#ifdef CATCH_USE_AFTER_FREE
+        text = const_cast<char *>("<use-after-free>");
+#endif
+    }
     
     inline operator const char * () const       { return text; }
     inline void clear()                         { setown(NULL); }