Merge pull request #6336 from rouault/coverity_scan_fixes

Another batch of Coverity scan fixes

mapagg.cpp

@@ -281,9 +281,6 @@ template<class PathStorage>
                               const mapserver::trans_affine& mtx,
                               PathStorage& path)
     {
-        FT_Vector   v_last;
-        FT_Vector   v_control;
-        FT_Vector   v_start;
         double x1, y1, x2, y2, x3, y3;
 
         FT_Vector*  point;
@@ -303,10 +300,9 @@ template<class PathStorage>
             last  = outline.contours[n];
             limit = outline.points + last;
 
-            v_start = outline.points[first];
-            v_last  = outline.points[last];
+            FT_Vector v_start = outline.points[first];
 
-            v_control = v_start;
+            FT_Vector v_control = v_start;
 
             point = outline.points + first;
             tags  = outline.tags  + first;
@@ -318,6 +314,8 @@ template<class PathStorage>
             // check first point to determine origin
             if( tag == FT_CURVE_TAG_CONIC)
             {
+                const FT_Vector v_last  = outline.points[last];
+
                 // first point is conic control.  Yes, this happens.
                 if(FT_CURVE_TAG(outline.tags[last]) == FT_CURVE_TAG_ON)
                 {
@@ -332,8 +330,6 @@ template<class PathStorage>
                     // for closure
                     v_start.x = (v_start.x + v_last.x) / 2;
                     v_start.y = (v_start.y + v_last.y) / 2;
-
-                    v_last = v_start;
                 }
                 point--;
                 tags--;

mapcluster.c

@@ -1386,7 +1386,8 @@ int msClusterLayerClose(layerObj *layer)
 
 #ifndef USE_CLUSTER_EXTERNAL
   /* switch back to the source layer vtable */
-  msInitializeVirtualTable(layer);
+  if( msInitializeVirtualTable(layer) != MS_SUCCESS )
+      return MS_FAILURE;
 #endif
 
   return MS_SUCCESS;

mapcrypto.c

@@ -193,7 +193,10 @@ int msGenerateEncryptionKey(unsigned char *k)
   srand( (unsigned int) time( NULL ));
 
   for(i=0; i<MS_ENCRYPTION_KEY_SIZE; i++)
+  {
+    /* coverity[dont_call] */
     k[i] = (unsigned char)rand();
+  }
 
   return MS_SUCCESS;
 }

mapdraw.c

@@ -552,7 +552,10 @@ imageObj *msDrawMap(mapObj *map, int querymap)
     if(lp->connectiontype == MS_WMS) {
 #ifdef USE_WMS_LYR
       if(MS_RENDERER_PLUGIN(image->format) || MS_RENDERER_RAWDATA(image->format))
+      {
+        assert(pasOWSReqInfo);
         status = msDrawWMSLayerLow(map->layerorder[i], pasOWSReqInfo, numOWSRequests, map, lp, image);
+      }
 
 #else
       status = MS_FAILURE;

mapkmlrenderer.h

@@ -41,73 +41,73 @@
 class KmlRenderer
 {
 private:
-  const char *pszLayerDescMetadata; /*if the kml_description is set*/
-  char **papszLayerIncludeItems;
-  int nIncludeItems;
-  char **papszLayerExcludeItems;
-  int nExcludeItems;
-  const char *pszLayerNameAttributeMetadata;
+  const char *pszLayerDescMetadata = nullptr; /*if the kml_description is set*/
+  char **papszLayerIncludeItems = nullptr;
+  int nIncludeItems = 0;
+  char **papszLayerExcludeItems = nullptr;
+  int nExcludeItems = 0;
+  const char *pszLayerNameAttributeMetadata = nullptr;
 
 protected:
 
   // map properties
-  int       Width, Height;
-  rectObj     MapExtent;
-  double      MapCellsize;
-  colorObj    BgColor;
-  char      MapPath[MS_MAXPATHLEN];
+  int       Width = 0, Height = 0;
+  rectObj     MapExtent = {0,0,0,0};
+  double      MapCellsize = 0;
+  colorObj    BgColor = {0,0,0,0};
+  char      MapPath[MS_MAXPATHLEN] = {0};
 
   // xml nodes pointers
-  xmlDocPtr XmlDoc;
-  xmlNodePtr  DocNode;
-  xmlNodePtr  LayerNode;
-  xmlNodePtr  GroundOverlayNode;
-
-  xmlNodePtr  PlacemarkNode;
-  xmlNodePtr  GeomNode;
-  xmlNodePtr  DescriptionNode;
-
-  int         CurrentShapeIndex;
-  int         CurrentDrawnShapeIndex;
-  char            *CurrentShapeName;
-  char    **Items;
-  int     NumItems;
-  int     DumpAttributes;
+  xmlDocPtr XmlDoc = nullptr;
+  xmlNodePtr  DocNode = nullptr;
+  xmlNodePtr  LayerNode = nullptr;
+  xmlNodePtr  GroundOverlayNode = nullptr;
+
+  xmlNodePtr  PlacemarkNode = nullptr;
+  xmlNodePtr  GeomNode = nullptr;
+  xmlNodePtr  DescriptionNode = nullptr;
+
+  int         CurrentShapeIndex = 0;
+  int         CurrentDrawnShapeIndex = 0;
+  char            *CurrentShapeName = nullptr;
+  char    **Items = nullptr;
+  int     NumItems = 0;
+  int     DumpAttributes = 0;
 
   // placemark symbology
-  hashTableObj  *StyleHashTable;
+  hashTableObj  *StyleHashTable = nullptr;
 
-  colorObj                LabelColor;
-  strokeStyleObj          *LineStyle;
-  int                     numLineStyle;
-  colorObj    PolygonColor;
+  colorObj                LabelColor = {0,0,0,0};
+  strokeStyleObj          *LineStyle = nullptr;
+  int                     numLineStyle = 0;
+  colorObj    PolygonColor = {0,0,0,0};
 
-  char      SymbolName[128];
-  char      SymbolUrl[128];
+  char      SymbolName[128] = {0};
+  char      SymbolUrl[128] = {0};
 
   enum      { NumSymbologyFlag = 4};
-  char      SymbologyFlag[NumSymbologyFlag];
+  char      SymbologyFlag[NumSymbologyFlag] = {0,0,0,0};
 
   enum      symbFlagsEnum { Label, Line, Polygon, Symbol };
 
-  int       FirstLayer;
+  int       FirstLayer = 0;
 
-  mapObj                  *map;
-  layerObj                *currentLayer;
+  mapObj                  *map = nullptr;
+  layerObj                *currentLayer = nullptr;
 
-  int       AltitudeMode;
-  int       Tessellate;
-  int       Extrude;
+  int       AltitudeMode = 0;
+  int       Tessellate = 0;
+  int       Extrude = 0;
 
   enum altitudeModeEnum { undefined, clampToGround, relativeToGround, absolute };
   /**True if elevation is taken from a feature attribute*/
-  bool mElevationFromAttribute;
+  bool mElevationFromAttribute = false;
   /**Attribute index of elevation (or -1 if elevation is not attribute driven*/
-  int mElevationAttributeIndex;
-  double mCurrentElevationValue;
+  int mElevationAttributeIndex = 0;
+  double mCurrentElevationValue = 0;
 
 
-  outputFormatObj *aggFormat;
+  outputFormatObj *aggFormat = nullptr;
 
 protected:
 

maplayer.c

@@ -1174,7 +1174,7 @@ int msLayerGetFeatureStyle(mapObj *map, layerObj *layer, classObj *c, shapeObj*
   if (layer->styleitem && layer->styleitemindex >=0) {
     stylestring = msStrdup(shape->values[layer->styleitemindex]);
   }
-  else if (strncasecmp(layer->styleitem,"javascript://",13) == 0) {
+  else if (layer->styleitem && strncasecmp(layer->styleitem,"javascript://",13) == 0) {
 #ifdef USE_V8_MAPSCRIPT
     char *filename = layer->styleitem+13;
 

mapogcsos.c

@@ -284,7 +284,7 @@ void msSOSAddPropertyNode(xmlNsPtr psNsSwe, xmlNsPtr psNsXLink, xmlNodePtr psPar
     pszValue = msOWSLookupMetadata(&(lp->metadata), "S",
                                    "observedproperty_name");
     if (pszValue)
-      psNode = xmlNewTextChild(psCompNode, psNsGml,
+      (void) xmlNewTextChild(psCompNode, psNsGml,
                            BAD_CAST "name", BAD_CAST pszValue);
 
     /* add components */

mappostgis.cpp

@@ -347,6 +347,8 @@ wkbSkipGeometry(wkbObj *w)
     }
     case WKB_POLYGON: {
       const int nrings = wkbReadInt(w);
+      if( nrings > (int)((w->size - (w->ptr - w->wkb)) / 4) )
+        return;
       for ( int i = 0; i < nrings; i++ ) {
         const int npoints = wkbReadInt(w);
         w->ptr += npoints * nCoordDim * sizeof(double);

mapprimitive.c

@@ -333,15 +333,16 @@ int msAddPointToLine(lineObj *line, pointObj *point )
   return MS_SUCCESS;
 }
 
-int msAddLine(shapeObj *p, lineObj *new_line)
+int msAddLine(shapeObj *p, const lineObj *new_line)
 {
   lineObj lineCopy;
 
   lineCopy.numpoints = new_line->numpoints;
   lineCopy.point = (pointObj *) malloc(new_line->numpoints*sizeof(pointObj));
   MS_CHECK_ALLOC(lineCopy.point, new_line->numpoints*sizeof(pointObj), MS_FAILURE);
 
-  memcpy( lineCopy.point, new_line->point, sizeof(pointObj) * new_line->numpoints );
+  if( new_line->point )
+      memcpy( lineCopy.point, new_line->point, sizeof(pointObj) * new_line->numpoints );
 
   // cppcheck-suppress memleak
   return msAddLineDirectly( p, &lineCopy );

mapquantization.c

@@ -495,10 +495,10 @@ mediancut( acolorhist_vector achv, int colors, int sum, unsigned char maxval, in
     register long r = 0, g = 0, b = 0, a = 0, sum = 0;
 
     for ( i = 0; i < clrs; ++i ) {
-      r += PAM_GETR( achv[indx + i].acolor ) * achv[indx + i].value;
-      g += PAM_GETG( achv[indx + i].acolor ) * achv[indx + i].value;
-      b += PAM_GETB( achv[indx + i].acolor ) * achv[indx + i].value;
-      a += PAM_GETA( achv[indx + i].acolor ) * achv[indx + i].value;
+      r += (long)PAM_GETR( achv[indx + i].acolor ) * achv[indx + i].value;
+      g += (long)PAM_GETG( achv[indx + i].acolor ) * achv[indx + i].value;
+      b += (long)PAM_GETB( achv[indx + i].acolor ) * achv[indx + i].value;
+      a += (long)PAM_GETA( achv[indx + i].acolor ) * achv[indx + i].value;
       sum += achv[indx + i].value;
     }
     if(sum>0) {

mapquery.c

@@ -30,6 +30,8 @@
 #include "mapserver.h"
 #include "mapows.h"
 
+#include "limits.h"
+
 /* This object is used by the various mapQueryXXXXX() functions. It stores
  * the total amount of shapes and their RAM footprint, when they are cached
  * in the resultCacheObj* of layers. This is the total number accross all queried
@@ -290,7 +292,7 @@ static int loadQueryResults(mapObj *map, FILE *stream)
 {
   int i, j, k, n=0;
 
-  if(1 != fread(&n, sizeof(int), 1, stream)) {
+  if(1 != fread(&n, sizeof(int), 1, stream) || n > INT_MAX - 1) {
     msSetError(MS_MISCERR,"failed to read query count from query file stream", "loadQueryResults()");
     return MS_FAILURE;
   }
@@ -445,7 +447,8 @@ static int loadQueryParams(mapObj *map, FILE *stream)
 
           if(fscanf(stream, "%d\n", &numlines) != 1) goto parse_error;
           for(i=0; i<numlines; i++) {
-            if(fscanf(stream, "%d\n", &numpoints) != 1 || numpoints<0) goto parse_error;
+            if(fscanf(stream, "%d\n", &numpoints) != 1 || numpoints<0 ||
+                numpoints > INT_MAX / (int)sizeof(pointObj)) goto parse_error;
 
             line.numpoints = numpoints;
             line.point = (pointObj *) msSmallMalloc(line.numpoints*sizeof(pointObj));

mapserv.c

@@ -195,6 +195,7 @@ int main(int argc, char *argv[])
     } else if( strncmp(argv[iArg], "QUERY_STRING=", 13) == 0 ) {
       /* Debugging hook... pass "QUERY_STRING=..." on the command-line */
       putenv( "REQUEST_METHOD=GET" );
+      /* coverity[tainted_string] */
       putenv( argv[iArg] );
     } else if (strcmp(argv[iArg], "--h") == 0 || strcmp(argv[iArg], "--help") == 0) {
       printf("Usage: mapserv [--help] [-v] [-nh] [QUERY_STRING=value]\n");

mapserver.h

@@ -2443,7 +2443,7 @@ extern "C" {
   MS_DLL_EXPORT int WARN_UNUSED msLineLabelPath(mapObj *map, imageObj *img, lineObj *p, textSymbolObj *ts, struct line_lengths *ll, struct label_follow_result *lfr, labelObj *lbl);
   MS_DLL_EXPORT int WARN_UNUSED msLineLabelPoint(mapObj *map, lineObj *p, textSymbolObj *ts, struct line_lengths *ll, struct label_auto_result *lar, labelObj *lbl, double resolutionfactor);
   MS_DLL_EXPORT int msPolygonLabelPoint(shapeObj *p, pointObj *lp, double min_dimension);
-  MS_DLL_EXPORT int msAddLine(shapeObj *p, lineObj *new_line);
+  MS_DLL_EXPORT int msAddLine(shapeObj *p, const lineObj *new_line);
   MS_DLL_EXPORT int msAddLineDirectly(shapeObj *p, lineObj *new_line);
   MS_DLL_EXPORT int msAddPointToLine(lineObj *line, pointObj *point );
   MS_DLL_EXPORT double msGetPolygonArea(shapeObj *p);

mapshape.c

@@ -1351,7 +1351,7 @@ void msSHPReadShape( SHPHandle psSHP, int hEntity, shapeObj *shape )
         shape->line[i].numpoints = nPoints - psSHP->panParts[i];
       else
         shape->line[i].numpoints = psSHP->panParts[i+1] - psSHP->panParts[i];
-      if (shape->line[i].numpoints <= 0) {
+      if (shape->line[i].numpoints <= 0 || shape->line[i].numpoints > nPoints) {
         msSetError(MS_SHPERR, "Corrupted .shp file : shape %d, shape->line[%d].numpoints=%d", "msSHPReadShape()",
                    hEntity, i, shape->line[i].numpoints);
         while(--i >= 0)

maptemplate.c

@@ -27,6 +27,8 @@
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
 
+#define NEED_IGNORE_RET_VAL
+
 #include "maptemplate.h"
 #include "maphash.h"
 #include "mapserver.h"
@@ -719,7 +721,7 @@ int processIfTag(char **pszInstr, hashTableObj *ht, int bLastPass)
       pszPatIn  = findTag(pszTmp+1, "if");
       pszPatOut = strstr(pszTmp+1, "[/if]");
 
-    } while (pszTmp != NULL && nInst > 0);
+    } while (nInst > 0);
 
     /* get the then string (if expression is true) */
     if(getInlineTag("if", pszStart, &pszThen) != MS_SUCCESS) {
@@ -790,9 +792,7 @@ int processIfTag(char **pszInstr, hashTableObj *ht, int bLastPass)
         return MS_FAILURE;
       }
 
-      if(pszIfTag)
-        free(pszIfTag);
-
+      free(pszIfTag);
       pszIfTag = NULL;
     }
 
@@ -3094,17 +3094,31 @@ char *generateLegendTemplate(mapservObj *mapserv)
   /* open template */
   if((stream = fopen(msBuildPath(szPath, mapserv->map->mappath, mapserv->map->legend.template), "r")) == NULL) {
     msSetError(MS_IOERR, "Error while opening template file.", "generateLegendTemplate()");
-    if(pszResult)
-      free(pszResult);
+    free(pszResult);
     pszResult=NULL;
     goto error;
   }
 
   fseek(stream, 0, SEEK_END);
-  length = ftell(stream);
+  long lengthLong = ftell(stream);
   rewind(stream);
+  if( lengthLong < 0 || lengthLong > INT_MAX - 1 )
+  {
+    msSetError(MS_IOERR, "Too large template file.", "generateLegendTemplate()");
+    free(pszResult);
+    pszResult=NULL;
+    goto error;
+  }
+  length = (int)lengthLong;
 
-  file = (char*)msSmallMalloc(length + 1);
+  file = (char*)malloc(length + 1);
+  if( file == NULL )
+  {
+    msSetError(MS_IOERR, "Cannot allocate memory for template file.", "generateLegendTemplate()");
+    free(pszResult);
+    pszResult=NULL;
+    goto error;
+  }
 
   /*
    * Read all the template file
@@ -4889,7 +4903,7 @@ char *msProcessQueryTemplate(mapObj *map, int bGenerateImages, char **names, cha
       msGenerateImages(mapserv, MS_TRUE, MS_FALSE);
 
     mapserv->sendheaders = MS_FALSE;
-    msReturnTemplateQuery(mapserv, mapserv->map->web.queryformat, &pszBuffer);
+    IGNORE_RET_VAL(msReturnTemplateQuery(mapserv, mapserv->map->web.queryformat, &pszBuffer));
 
     mapserv->map = NULL;
     mapserv->request->ParamNames = mapserv->request->ParamValues = NULL;