Przeglądaj źródła

HPCC-16455 Fix a few more unbounded strcpy examples

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 8 lat temu
rodzic
commit
f46ba756b7

+ 2 - 1
esp/clients/wsecl/ws_ecl_client.cpp

@@ -56,7 +56,8 @@ CClientWsEclService::~CClientWsEclService()
 
 void CClientWsEclService::addServiceUrl(const char * url)
 {
-    strcpy(m_url, url);
+    strncpy(m_url, url, sizeof(m_url));
+    m_url[sizeof(m_url)-1] = 0;
 }
 
 void CClientWsEclService::removeServiceUrl(const char *url)

+ 18 - 25
esp/services/ws_dfu/ws_dfuService.cpp

@@ -2555,22 +2555,15 @@ bool CWsDfuEx::checkDescription(const char *description, const char *description
     if (descriptionFilter[len - 1] == '*')
         filterType += 2;
 
-    char descFilter[256];
+    StringBuffer descFilter;
     if (filterType < 1)
-        strcpy(descFilter, descriptionFilter);
+        descFilter.append(descriptionFilter);
     else if (filterType < 2)
-        strcpy(descFilter, descriptionFilter+1);
+        descFilter.append(descriptionFilter+1);
     else if (filterType < 3)
-    {
-        strncpy(descFilter, descriptionFilter, len - 1);
-        descFilter[len -2] = 0;
-    }
+        descFilter.append(len-1, descriptionFilter);
     else
-    {
-        strncpy(descFilter, descriptionFilter+1, len - 2);
-        descFilter[len -3] = 0;
-    }
-
+        descFilter.append(len-2, descriptionFilter+1);
 
     const char *pos = strstr(description, descFilter);
     if (!pos)
@@ -2650,10 +2643,10 @@ void CWsDfuEx::getAPageOfSortedLogicalFile(IEspContext &context, IUserDescriptor
         wuTo.appendf("%4d-%02d-%02d %02d:%02d:%02d",year,month,day,hour,minute,second);
     }
 
-    char sortBy[256];
+    StringBuffer sortBy;
     if(req.getSortby() && *req.getSortby())
     {
-        strcpy(sortBy, req.getSortby());
+        sortBy.append(req.getSortby());
     }
 
     unsigned pagesize = req.getPageSize();
@@ -2680,22 +2673,22 @@ void CWsDfuEx::getAPageOfSortedLogicalFile(IEspContext &context, IUserDescriptor
         displayEnd = nFirstN;
         if (!stricmp(sFirstNType, "newest"))
         {
-            strcpy(sortBy, "Modified");
+            sortBy.set("Modified");
             descending = true;
         }
         else if (!stricmp(sFirstNType, "oldest"))
         {
-            strcpy(sortBy, "Modified");
+            sortBy.set("Modified");
             descending = false;
         }
         else if (!stricmp(sFirstNType, "largest"))
         {
-            strcpy(sortBy, "FileSize");
+            sortBy.set("FileSize");
             descending = true;
         }
         else if (!stricmp(sFirstNType, "smallest"))
         {
-            strcpy(sortBy, "FileSize");
+            sortBy.set("FileSize");
             descending = false;
         }
         pagesize = nFirstN;
@@ -3042,41 +3035,41 @@ void CWsDfuEx::getAPageOfSortedLogicalFile(IEspContext &context, IUserDescriptor
     if (ParametersForFilters.length() > 0)
         resp.setFilters(ParametersForFilters.str());
 
-    sortBy[0] = 0;
+    sortBy.clear();
     descending = false;
     if ((req.getFirstN() > 0) && req.getFirstNType() && *req.getFirstNType())
     {
         const char *sFirstNType = req.getFirstNType();
         if (!stricmp(sFirstNType, "newest"))
         {
-            strcpy(sortBy, "Modified");
+            sortBy.set("Modified");
             descending = true;
         }
         else if (!stricmp(sFirstNType, "oldest"))
         {
-            strcpy(sortBy, "Modified");
+            sortBy.set("Modified");
             descending = false;
         }
         else if (!stricmp(sFirstNType, "largest"))
         {
-            strcpy(sortBy, "FileSize");
+            sortBy.set("FileSize");
             descending = true;
         }
         else if (!stricmp(sFirstNType, "smallest"))
         {
-            strcpy(sortBy, "FileSize");
+            sortBy.set("FileSize");
             descending = false;
         }
 
     }
     else if (req.getSortby() && *req.getSortby())
     {
-        strcpy(sortBy, req.getSortby());
+        sortBy.set(req.getSortby());
         if (req.getDescending())
             descending = req.getDescending();
     }
 
-    if (sortBy && *sortBy)
+    if (sortBy.length())
     {
         resp.setSortby(sortBy);
         resp.setDescending(descending);

+ 2 - 5
esp/services/ws_machine/ws_machineServiceMetrics.cpp

@@ -649,13 +649,10 @@ bool Cws_machineEx::onGetMetrics(IEspContext &context, IEspMetricsRequest &req,
             if (!ep || !*ep)
                 continue;
 
-            char ip[32];
-            strcpy(ip, ep);
+            StringBuffer ip(ep);
             const char* ip0 = strchr(ep, ':');
             if (ip0)
-            {
-                ip[ip0 - ep] = 0;
-            }
+                ip.setLength(ip0 - ep);
 
             Owned<CMetricsParam> pMetricsParam = new CMetricsParam(ip);
             Owned<IPropertyTreeIterator> metrics = endpoint.getElements("Metrics/Metric");

+ 5 - 17
esp/services/ws_workunits/ws_workunitsAuditLogs.cpp

@@ -491,40 +491,28 @@ bool readECLWUCurrentJob(const char* curjob, const char* clusterName, const char
     wuidStr.clear().append(eptr - bptr, bptr);
 
     //graph number
-    char graph[32];
     bptr = eptr + 1;
     eptr = strchr(bptr, ',');
     if(!eptr)
         return false;
 
-    len = eptr - bptr;
     if (bptr[0] == 'g' && len > 5)
-    {
         bptr += 5;
-        len = eptr - bptr;
-    }
 
-    strncpy(graph, bptr, len);
-    graph[len] = 0;
     graphStr.clear().append(eptr - bptr, bptr);
 
     if (!stricmp(action, "start") || !stricmp(action, "stop"))
         return true;
 
     //subgraph number
-    char subgraph[32];
+    StringBuffer subgraph;
     bptr = eptr + 1;
     eptr = strchr(bptr, ',');
-    if(!eptr)
-    {
-        strcpy(subgraph, bptr);
-    }
+    if (!eptr)
+        subgraph.append(bptr);
     else
-    {
-        len = eptr - bptr;
-        strncpy(subgraph, bptr, len);
-        subgraph[len] = 0;
-    }
+        subgraph.append(eptr - bptr, bptr);
+
     subGraphStr.clear().append(subgraph);
 
     return true;

+ 6 - 4
system/jlib/jstring.cpp

@@ -216,13 +216,15 @@ StringBuffer & StringBuffer::append(const char * value)
     return *this;
 }
 
-StringBuffer & StringBuffer::append(unsigned len, const char * value)
+StringBuffer & StringBuffer::append(size_t len, const char * value)
 {
     if (len)
     {
-        ensureCapacity(len);
-        memcpy(buffer + curLen, value, len);
-        curLen += len;
+        unsigned truncLen = (unsigned)len;
+        assertex(truncLen == len); // MORE: StringBuffer should use size_t throughout
+        ensureCapacity(truncLen);
+        memcpy(buffer + curLen, value, truncLen);
+        curLen += truncLen;
     }
     return *this;
 }

+ 1 - 1
system/jlib/jstring.hpp

@@ -56,7 +56,7 @@ public:
     StringBuffer &  append(const char * value);
     StringBuffer &  append(const unsigned char * value);
     StringBuffer &  append(const IAtom * value);
-    StringBuffer &  append(unsigned len, const char * value);
+    StringBuffer &  append(size_t len, const char * value);
     StringBuffer &  append(const char * value, int offset, int len);
     StringBuffer &  append(double value);
     StringBuffer &  append(float value);

+ 3 - 7
system/mp/test/mptest.cpp

@@ -709,19 +709,15 @@ int main(int argc, char* argv[])
 
         INode *nodes[1000];
 
-        bool has_argfile = false;
-        StringBuffer argfile;
+        const char * argfile = nullptr;
         if (argc > 3)
         {
             if (strcmp(argv[2], "-f") == 0)
-            {
-                has_argfile = true;
-                argfile.append(argv[3]);
-            }
+                argfile = argv[3];
         }
 
         int i = 1;
-        if (has_argfile)
+        if (argfile)
         {
             char hoststr[256] = { "" };
             FILE *fp = fopen(argfile, "r");