Browse Source

HPCC-10552 Fix various leaks and out of bound accesses

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 11 years ago
parent
commit
6ce090c70e

+ 11 - 3
common/deftype/defvalue.cpp

@@ -446,7 +446,7 @@ void MemoryValue::deserialize(MemoryBuffer &src)
     void *mem = checked_malloc(size, DEFVALUE_MALLOC_FAILED);
     assertex(mem);
     src.read(size, mem);
-    val.set(size, mem);
+    val.setOwn(size, mem);
 }
 
 int MemoryValue::rangeCompare(ITypeInfo * targetType)
@@ -801,6 +801,12 @@ void UnicodeAttr::set(UChar const * _text, unsigned _len)
     text[_len] = 0x0000;
 }
 
+void UnicodeAttr::setown(UChar * _text)
+{
+    free(text);
+    text = _text;
+}
+
 VarUnicodeValue::VarUnicodeValue(unsigned len, const UChar * v, ITypeInfo * _type) : CValue(_type)
 {
     unsigned typeLen = type->getStringLen();
@@ -958,7 +964,7 @@ void VarUnicodeValue::deserialize(MemoryBuffer & src)
     src.read(len);
     UChar * buff = (UChar *) checked_malloc(len*2, DEFVALUE_MALLOC_FAILED);
     src.read(len*2, buff);
-    val.set(buff, len);
+    val.setown(buff);
 }
 
 IValue *createVarUnicodeValue(char const * value, unsigned size, char const * locale, bool utf8, bool unescape)
@@ -2914,7 +2920,7 @@ IValue * substringValue(IValue * v, IValue * lower, IValue * higher)
     unsigned low = lower ? (unsigned)lower->getIntValue() : 0;
     unsigned high = higher ? (unsigned)higher->getIntValue() : srcLen;
 
-    unsigned retLen;
+    unsigned retLen = 0;
     void * retPtr;
     ITypeInfo * retType = NULL;
     switch (type->getTypeCode())
@@ -2942,6 +2948,8 @@ IValue * substringValue(IValue * v, IValue * lower, IValue * higher)
     case type_utf8:
         rtlUtf8SubStrFTX(retLen, *(char * *)&retPtr, srcLen, (const char *)raw, low, high);
         break;
+    default:
+        UNIMPLEMENTED;
     }
 
     if (retType == NULL)

+ 1 - 0
common/deftype/defvalue.ipp

@@ -158,6 +158,7 @@ public:
     UChar const * get(void) const { return text; }
     size32_t length() const { return text ? rtlUnicodeStrlen(text) : 0; }
     void set(UChar const * _text, unsigned _len);
+    void setown(UChar * _text);
 
 private:
     UChar * text;

+ 1 - 0
dali/datest/datest.cpp

@@ -929,6 +929,7 @@ void TestRemoteFile(unsigned part,unsigned of)
 
     infile->Release();
     outfile->Release();
+    free(buffer);
 }
 
 

+ 0 - 4
dali/datest/dfuwutest.cpp

@@ -39,8 +39,6 @@ void testProgressUpdate()
 
 void testAbort(const char *wuid)
 {
-    getDFUWorkUnitFactory();
-
     Owned<IDFUWorkUnitFactory> factory = getDFUWorkUnitFactory();
     Owned<IConstDFUWorkUnit> wu = factory->openWorkUnit(wuid,false);
     if (wu) {
@@ -58,8 +56,6 @@ StringBuffer& constructFileMask(const char* filename, StringBuffer& filemask)
 
 void testProgressMonitor(const char *wuid)
 {
-    getDFUWorkUnitFactory();
-
     Owned<IDFUWorkUnitFactory> factory = getDFUWorkUnitFactory();
     Owned<IConstDFUWorkUnit> wu = factory->openWorkUnit(wuid,false);
     class cProgressMon: public CInterface, implements IDFUprogressSubscriber

+ 3 - 0
dali/dfuXRefLib/dfuxreflib.cpp

@@ -1114,6 +1114,7 @@ public:
             }
             out.append('[').append(pna.item(sorted[i2])+1).append(']');
         }
+        delete [] sorted;
     }
 
     IPropertyTree *addFileBranch(IPropertyTree *dst,unsigned flags)
@@ -1562,6 +1563,8 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch
                                     oldentry->crosslink.setown(entry);
 #endif
                                 }
+                                else
+                                    entry->Release();
                             }
                             else {
                                 manager.filemap.add(*entry);

+ 1 - 1
dali/sasha/saxref.cpp

@@ -247,7 +247,7 @@ struct cDirDesc
     {
         size32_t sl = strlen(_name);
         if (sl>255) {
-            WARNLOG(LOGPFX "Directory name %s longer than 255 chars, truncating",name);
+            WARNLOG(LOGPFX "Directory name %s longer than 255 chars, truncating",_name);
             sl = 255;
         }
         name = (byte *)mem.alloc(sl+1);

+ 4 - 0
deployment/deployutils/deployutils.cpp

@@ -2797,6 +2797,8 @@ const char* getUniqueName(const IPropertyTree* pEnv, StringBuffer& sName, const
       if (len > 0 && endsWith(sPrefix.str(), "_")) //ends with '_'
         sPrefix = sPrefix.remove(sPrefix.length() - 1, 1); //lose it
     }
+
+    free(pszNum);
   }
 
   StringBuffer xpath;
@@ -2840,6 +2842,8 @@ const char* getUniqueName2(const IPropertyTree* pEnv, StringBuffer& sName, const
       if (len > 0 && endsWith(sPrefix.str(), "_")) //ends with '_'
         sPrefix = sPrefix.remove(sPrefix.length() - 1, 1); //lose it
     }
+
+    free(pszNum);
   }
 
   StringBuffer xpath;

+ 1 - 1
ecl/eclagent/eclagent.ipp

@@ -937,7 +937,7 @@ public:
     EclSubGraph(IAgentContext & _agent, EclGraph &parent, EclSubGraph * _owner, unsigned subGraphSeqNo, bool enableProbe, CHThorDebugContext * _debugContext, IProbeManager * _probeManager);
     IMPLEMENT_IINTERFACE
 
-    void createFromXGMML(EclGraph * graph, ILoadedDllEntry * dll, IPropertyTree * xgmml, unsigned * subGraphSeqNo, EclSubGraph * resultsGraph);
+    void createFromXGMML(EclGraph * graph, ILoadedDllEntry * dll, IPropertyTree * xgmml, unsigned & subGraphSeqNo, EclSubGraph * resultsGraph);
     void execute(const byte * parentExtract);
     void executeChild(const byte * parentExtract, IHThorGraphResults * results, IHThorGraphResults * _graphLoopResults);
     void executeLibrary(const byte * parentExtract, IHThorGraphResults * results);

+ 6 - 4
ecl/eclagent/eclgraph.cpp

@@ -379,6 +379,8 @@ bool EclGraphElement::alreadyUpToDate(IAgentContext & agent)
             helper->getUpdateCRCs(eclCRC, totalCRC);
             break;
         }
+    default:
+        UNIMPLEMENTED;
     }
 
     Owned<ILocalOrDistributedFile> ldFile = agent.resolveLFN(filename.get(), "Read", true, false, false);
@@ -776,7 +778,7 @@ EclSubGraph::EclSubGraph(IAgentContext & _agent, EclGraph & _parent, EclSubGraph
     isChildGraph = false;
 }
 
-void EclSubGraph::createFromXGMML(EclGraph * graph, ILoadedDllEntry * dll, IPropertyTree * node, unsigned * subGraphSeqNo, EclSubGraph * resultsGraph)
+void EclSubGraph::createFromXGMML(EclGraph * graph, ILoadedDllEntry * dll, IPropertyTree * node, unsigned & subGraphSeqNo, EclSubGraph * resultsGraph)
 {
     xgmml.set(node->queryPropTree("att/graph"));
 
@@ -803,9 +805,9 @@ void EclSubGraph::createFromXGMML(EclGraph * graph, ILoadedDllEntry * dll, IProp
         {
             Owned<IProbeManager> childProbe;
             if (probeManager)
-                childProbe.setown(probeManager->startChildGraph(*subGraphSeqNo, NULL));
+                childProbe.setown(probeManager->startChildGraph(subGraphSeqNo, NULL));
 
-            Owned<EclSubGraph> subgraph = new EclSubGraph(*agent, *graph, this, *subGraphSeqNo++, probeEnabled, debugContext, probeManager);
+            Owned<EclSubGraph> subgraph = new EclSubGraph(*agent, *graph, this, subGraphSeqNo++, probeEnabled, debugContext, probeManager);
             subgraph->createFromXGMML(graph, dll, &cur, subGraphSeqNo, resultsGraph);
             if (probeManager)
                 probeManager->endChildGraph(childProbe, NULL);
@@ -1129,7 +1131,7 @@ void EclGraph::createFromXGMML(ILoadedDllEntry * dll, IPropertyTree * xgmml, boo
             childProbe.setown(probeManager->startChildGraph(subGraphSeqNo, NULL));
 
         Owned<EclSubGraph> subgraph = new EclSubGraph(*agent, *this, NULL, subGraphSeqNo++, enableProbe, debugContext, probeManager);
-        subgraph->createFromXGMML(this, dll, &iter->query(), &subGraphSeqNo, NULL);
+        subgraph->createFromXGMML(this, dll, &iter->query(), subGraphSeqNo, NULL);
         if (probeManager)
             probeManager->endChildGraph(childProbe, NULL);
 

+ 3 - 0
ecl/eclplus/eclplus.cpp

@@ -139,9 +139,12 @@ IEclPlusHelper * createEclPlusHelper(IProperties * globals)
         }
         else
         {
+            ::Release(format);
             throw MakeStringException(-1, "unknown action");
         }
     }
+    else
+        ::Release(format);
     return helper;
 }
 

+ 3 - 3
esp/bindings/http/platform/httpprot.cpp

@@ -182,10 +182,10 @@ bool CHttpProtocol::notifySelected(ISocket *sock,unsigned selected)
                             if(ci && ci->IsShared())
                                 accepted->Release();
                         }
-                        delete holder;
+                        delete [] holder;
                         throw;
                     }
-                    delete holder;
+                    delete [] holder;
                 }
                 else
                 {
@@ -346,7 +346,7 @@ bool CSecureHttpProtocol::notifySelected(ISocket *sock,unsigned selected)
                         holder[3] = (void*)&useSSL;
                         holder[4] = (void*)m_ssctx.get();
                         http_thread_pool->start((void*)holder);
-                        delete holder;
+                        delete [] holder;
                     }
                     else
                     {

+ 1 - 1
esp/bindings/http/platform/httptransport.cpp

@@ -864,7 +864,7 @@ int CHttpMessage::send()
                 break;
             }
         }
-        delete buffer;
+        delete [] buffer;
     }
 
     return retcode;

+ 2 - 2
esp/bindings/http/platform/mime.cpp

@@ -312,7 +312,7 @@ void CMimeMultiPart::unserialize(const char* contenttype, int text_length, const
         ptr = Utils::getWord(ptr, oneword, "; ");
     }
 
-    delete typebuf;
+    delete [] typebuf;
 
     int oneline_len = 0;
     int cur_pos = 0;
@@ -488,7 +488,7 @@ void CMimeMultiPart::parseContentType(const char* contenttype)
         ptr = Utils::getWord(ptr, oneword, "; ");
     }
 
-    delete typebuf;
+    delete [] typebuf;
 
     return;
 }

+ 2 - 2
esp/services/WsDeploy/WsDeployService.cpp

@@ -153,7 +153,7 @@ void substituteParameters(const IPropertyTree* pEnv, const char *xpath, IPropert
         StringBuffer sb(xpath2);
         int pos = xpath2.indexOf(']');
         if (pos != -1)
-          sb.clear().append(xpath2.substring(0, pos)->toCharArray());
+          sb.clear().append(xpath2.toCharArray(), 0, pos);
 
         result.append('\"');
         result.append(pNode->queryProp(sb.str()));
@@ -3853,7 +3853,7 @@ bool CWsDeployFileInfo::getBuildServerDirs(IEspContext &context, IEspGetBuildSer
   else 
   {
     Owned<IFile> inFiles = NULL;
-    IPropertyTree* pParentNode = createPTree("BuildServerComps");
+    Owned<IPropertyTree> pParentNode = createPTree("BuildServerComps");
 
     if (!strcmp(cmd, "Release"))
       sourceDir.append(PATHSEPCHAR).append("release");

+ 1 - 0
esp/test/httptest/httptest.cpp

@@ -421,6 +421,7 @@ int HttpClient::sendRequest(StringBuffer& req)
             if(m_stat.slowest < stat.slowest)
                 m_stat.slowest = stat.slowest;
         }
+        delete [] thrdlist;
     }
     else
         sendRequest(m_times, m_stat, req);

+ 1 - 0
esp/tools/soapplus/http.cpp

@@ -1006,6 +1006,7 @@ int HttpClient::sendStressRequests(HttpStat* overall_stat)
                 overall_stat->slowest = stat->slowest;
         }
     }
+    delete [] thrdlist;
 
     return 0;
 }

+ 2 - 0
esp/tools/soapplus/xmldiff.cpp

@@ -709,6 +709,8 @@ int CXmlDiff::countDiff(const char* xpath, IPropertyTree* t1, IPropertyTree* t2,
                 {
                     int estimate = chld_total*unmatched/(i+1) + 2*chld_diff;
                     m_diffcountcache[key] = estimate;
+                    delete[] ptrs1;
+                    delete[] ptrs2;
                     return estimate;
                 }
             }

+ 1 - 0
roxie/udplib/uttest.cpp

@@ -435,6 +435,7 @@ public:
 
     ~SortMaster()
     {
+        delete [] nodeData;
         delete [] nextNode;
         delete [] receiveSem;
         delete [] receivesCompleted;

+ 8 - 20
rtl/eclrtl/eclrtl.cpp

@@ -636,14 +636,6 @@ void rtl_ls82en(size32_t l, char * t, __int64 val)
 //=============================================================================
 // Numeric conversion functions... - unknown length ebcdic target
 
-#define intToUnknownEbcdicStringBody() \
-    unsigned alen = numtostr(astr, val); \
-    rtlStrToEStrX(elen,estr,alen,astr); \
-    char * result = (char *)rtlMalloc(elen); \
-    memcpy(result, estr, elen); \
-    l = elen; \
-    t = result;
-
 #if defined _MSC_VER
 #pragma warning(push)
 #pragma warning(disable:4700)
@@ -651,33 +643,29 @@ void rtl_ls82en(size32_t l, char * t, __int64 val)
 void rtl_l42ex(size32_t & l, char * & t, unsigned val)
 {
     char astr[20];
-    char * estr;
-    unsigned elen;
-    intToUnknownEbcdicStringBody();
+    unsigned alen = numtostr(astr, val);
+    rtlStrToEStrX(l,t,alen,astr);
 }
 
 void rtl_l82ex(size32_t & l, char * & t, unsigned __int64 val)
 {
     char astr[40];
-    char * estr;
-    unsigned elen;
-    intToUnknownEbcdicStringBody();
+    unsigned alen = numtostr(astr, val);
+    rtlStrToEStrX(l,t,alen,astr);
 }
 
 void rtl_ls42ex(size32_t & l, char * & t, int val)
 {
     char astr[20];
-    char * estr;
-    unsigned elen;
-    intToUnknownEbcdicStringBody();
+    unsigned alen = numtostr(astr, val);
+    rtlStrToEStrX(l,t,alen,astr);
 }
 
 void rtl_ls82ex(size32_t & l, char * & t, __int64 val)
 {
     char astr[40];
-    char * estr;
-    unsigned elen;
-    intToUnknownEbcdicStringBody();
+    unsigned alen = numtostr(astr, val);
+    rtlStrToEStrX(l,t,alen,astr);
 }
 #ifdef _MSC_VER
 #pragma warning(pop)

+ 1 - 1
system/jlib/jmisc.cpp

@@ -517,7 +517,7 @@ bool invoke_program(const char *command_line, DWORD &runcode, bool wait, const c
     {
         if (outfile&&*outfile) {
             int outh = open(outfile, O_CREAT|O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR);
-            if(outh > 0)
+            if(outh >= 0)
             {
                 dup2(outh, STDOUT_FILENO);
                 dup2(outh, STDERR_FILENO);