[mapserver-commits] r9199 - in branches/branch-5-0/mapserver: . tests

svn at osgeo.org svn at osgeo.org
Tue Jul 21 15:34:37 EDT 2009


Author: aboudreault
Date: 2009-07-21 15:34:36 -0400 (Tue, 21 Jul 2009)
New Revision: 9199

Modified:
   branches/branch-5-0/mapserver/HISTORY.TXT
   branches/branch-5-0/mapserver/mapfile.c
   branches/branch-5-0/mapserver/mapquery.c
   branches/branch-5-0/mapserver/mapserv.c
   branches/branch-5-0/mapserver/mapserver.h
   branches/branch-5-0/mapserver/mapsymbol.c
   branches/branch-5-0/mapserver/maptemplate.c
   branches/branch-5-0/mapserver/maptemplate.h
   branches/branch-5-0/mapserver/tests/symbols.txt
Log:
Fixed several security issues (branch-5-0) (#2939, #2941, #2942, #2943, #2944)

Modified: branches/branch-5-0/mapserver/HISTORY.TXT
===================================================================
--- branches/branch-5-0/mapserver/HISTORY.TXT	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/HISTORY.TXT	2009-07-21 19:34:36 UTC (rev 9199)
@@ -13,6 +13,8 @@
 Current Version (SVN branch, may never be released):
 ----------------------------------------------------
 
+- Fixed several security issues found in an audit of the CGI application (#2939, #2941, #2942, #2944)
+
 - Fix for CVE-2009-0840 security vulnerability (#2943)
 
 Version 5.0.3 (2008-06-04)

Modified: branches/branch-5-0/mapserver/mapfile.c
===================================================================
--- branches/branch-5-0/mapserver/mapfile.c	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/mapfile.c	2009-07-21 19:34:36 UTC (rev 9199)
@@ -4228,11 +4228,21 @@
 static int loadMapInternal(mapObj *map)
 {
   int i,j,k;
+  int foundMapToken=MS_FALSE; 
+  int token;
 
   for(;;) {
 
-    switch(msyylex()) {   
+    token = msyylex(); 
 
+    if(!foundMapToken && token != MAP) { 
+      msSetError(MS_IDENTERR, "First token must be MAP, this doesn't look like a mapfile.", "msLoadMap()"); 
+      return(MS_FAILURE); 
+    }
+
+    switch(token) {
+
+
     case(CONFIG):
     {
         char *key=NULL, *value=NULL;
@@ -4358,6 +4368,7 @@
       if(loadLegend(&(map->legend), map) == -1) return MS_FAILURE;
       break;
     case(MAP):
+      foundMapToken = MS_TRUE; 
       break;   
     case(MAXSIZE):
       if(getInteger(&(map->maxsize)) == -1) return MS_FAILURE;

Modified: branches/branch-5-0/mapserver/mapquery.c
===================================================================
--- branches/branch-5-0/mapserver/mapquery.c	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/mapquery.c	2009-07-21 19:34:36 UTC (rev 9199)
@@ -120,6 +120,11 @@
     return(MS_FAILURE);
   }
 
+  /* 
+  ** Make sure the file at least has the right extension. 
+  */ 
+  if(msEvalRegex("\\.qy$", filename) != MS_TRUE) return MS_FAILURE; 
+
   stream = fopen(filename, "rb");
   if(!stream) {
     msSetError(MS_IOERR, "(%s)", "msLoadQuery()", filename);

Modified: branches/branch-5-0/mapserver/mapserv.c
===================================================================
--- branches/branch-5-0/mapserver/mapserv.c	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/mapserv.c	2009-07-21 19:34:36 UTC (rev 9199)
@@ -197,10 +197,23 @@
   } else {
     if(getenv(msObj->request->ParamValues[i])) /* an environment references the actual file to use */
       map = msLoadMap(getenv(msObj->request->ParamValues[i]), NULL);
-    else
+    else {
+      /* by here we know the request isn't for something in an environment variable */
+      if(getenv("MS_MAP_NO_PATH")) {
+        msSetError(MS_WEBERR, "Mapfile not found in environment variables and this server is not configured for full paths.", "loadMap()");
+ 	writeError();
+      }
+ 
+      if(getenv("MS_MAP_PATTERN") && msEvalRegex(getenv("MS_MAP_PATTERN"), msObj->request->ParamValues[i]) != MS_TRUE) {
+        msSetError(MS_WEBERR, "Parameter 'map' value fails to validate.", "loadMap()");
+       writeError();
+      }
+ 
+      /* ok to try to load now */
       map = msLoadMap(msObj->request->ParamValues[i], NULL);
+    }
   }
-
+  
   if(!map) writeError();
 
   /* check for any %variable% substitutions here, also do any map_ changes, we do this here so WMS/WFS  */
@@ -357,6 +370,10 @@
     }
 
     if(strcasecmp(msObj->request->ParamNames[i],"id") == 0) {
+      if(msEvalRegex(IDPATTERN, msObj->request->ParamValues[i]) == MS_FALSE) { 
+        msSetError(MS_WEBERR, "Parameter 'id' value fails to validate.", "loadMap()"); 
+        writeError(); 
+      } 
       strncpy(msObj->Id, msObj->request->ParamValues[i], IDSIZE);
       continue;
     }
@@ -1207,7 +1224,7 @@
     loadForm();
  
     if(msObj->SaveMap) {
-      sprintf(buffer, "%s%s%s.map", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id);
+      snprintf(buffer, sizeof(buffer), "%s%s%s.map", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id);
       if(msSaveMap(msObj->Map, buffer) == -1) writeError();
     }
 
@@ -1585,7 +1602,7 @@
         if(msReturnTemplateQuery(msObj, msObj->Map->web.queryformat, NULL) != MS_SUCCESS) writeError();
           
         if(msObj->SaveQuery) {
-          sprintf(buffer, "%s%s%s%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_QUERY_EXTENSION);
+          snprintf(buffer, sizeof(buffer), "%s%s%s%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_QUERY_EXTENSION);
           if((status = msSaveQuery(msObj->Map, buffer)) != MS_SUCCESS) return status;
         }
       }

Modified: branches/branch-5-0/mapserver/mapserver.h
===================================================================
--- branches/branch-5-0/mapserver/mapserver.h	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/mapserver.h	2009-07-21 19:34:36 UTC (rev 9199)
@@ -163,8 +163,10 @@
 /* General defines, not wrapable */
 #ifndef SWIG
 #define MS_DEFAULT_MAPFILE_PATTERN "\\.map$"
-#define MS_TEMPLATE_EXPR "\\.(jsp|asp|cfm|xml|wml|html|htm|shtml|phtml|php|svg)$"
 
+#define MS_TEMPLATE_MAGIC_STRING "MapServer Template"
+#define MS_TEMPLATE_EXPR "\\.(xml|wml|html|htm|svg|kml|gml|js|tmpl)$"
+
 #define MS_INDEX_EXTENSION ".qix"
 #define MS_QUERY_EXTENSION ".qy"
 

Modified: branches/branch-5-0/mapserver/mapsymbol.c
===================================================================
--- branches/branch-5-0/mapserver/mapsymbol.c	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/mapsymbol.c	2009-07-21 19:34:36 UTC (rev 9199)
@@ -616,7 +616,7 @@
 int msLoadSymbolSet(symbolSetObj *symbolset, mapObj *map)
 {
     int retval = MS_FAILURE;
-    
+
     msAcquireLock( TLOCK_PARSER );
     retval = loadSymbolSet( symbolset, map );
     msReleaseLock( TLOCK_PARSER );
@@ -631,6 +631,9 @@
   int status=1;
   char szPath[MS_MAXPATHLEN], *pszSymbolPath=NULL;
 
+  int foundSymbolSetToken=MS_FALSE;
+  int token;
+
   if(!symbolset) {
     msSetError(MS_SYMERR, "Symbol structure unallocated.", "loadSymbolSet()");
     return(-1);
@@ -657,7 +660,15 @@
   ** Read the symbol file
   */
   for(;;) {
-    switch(msyylex()) {
+
+    token = msyylex();
+
+    if(!foundSymbolSetToken && token != SYMBOLSET) {
+      msSetError(MS_IDENTERR, "First token must be SYMBOLSET, this doesn't look like a symbol file.", "msLoadSymbolSet()");
+      return(-1);
+    }
+
+    switch(token) {
     case(END):
     case(EOF):      
       status = 0;
@@ -673,6 +684,7 @@
           symbolset->numsymbols++;
       break;
     case(SYMBOLSET):
+      foundSymbolSetToken = MS_TRUE;
       break;
     default:
       msSetError(MS_IDENTERR, "Parsing error near (%s):(line %d)", "loadSymbolSet()", msyytext, msyylineno);

Modified: branches/branch-5-0/mapserver/maptemplate.c
===================================================================
--- branches/branch-5-0/mapserver/maptemplate.c	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/maptemplate.c	2009-07-21 19:34:36 UTC (rev 9199)
@@ -40,6 +40,20 @@
 
 char *processLine(mapservObj* msObj, char* instr, int mode);
 
+static int isValidTemplate(FILE *stream, const char *filename)
+{
+  char buffer[MS_BUFFER_LENGTH];
+
+  if(fgets(buffer, MS_BUFFER_LENGTH, stream) != NULL) {
+    if(!msCaseFindSubstring(buffer, MS_TEMPLATE_MAGIC_STRING)) {
+      msSetError(MS_WEBERR, "Missing magic string, %s doesn't look like a MapServer template.", "isValidTemplate()", filename);
+      return MS_FALSE;
+    }
+  }
+
+  return MS_TRUE;
+}
+
 /*
  * Redirect to (only use in CGI)
  * 
@@ -200,7 +214,7 @@
       img = msDrawMap(msObj->Map, MS_TRUE);
       if(!img) return MS_FAILURE;
 
-      snprintf(buffer, 1024, "%s%s%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
+      snprintf(buffer, sizeof(buffer), "%s%s%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
 
       status = msSaveImage(msObj->Map, img, buffer);
       if(status != MS_SUCCESS) return status;
@@ -211,7 +225,7 @@
       {
          img = msDrawLegend(msObj->Map, MS_FALSE);
          if(!img) return MS_FAILURE;
-         snprintf(buffer, 1024, "%s%sleg%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
+         snprintf(buffer, sizeof(buffer), "%s%sleg%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
          status = msSaveImage(msObj->Map, img, buffer);
          if(status != MS_SUCCESS) return status;
          msFreeImage(img);
@@ -221,7 +235,7 @@
       {
          img = msDrawScalebar(msObj->Map);
          if(!img) return MS_FAILURE;
-         snprintf(buffer, 1024, "%s%ssb%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
+         snprintf(buffer, sizeof(buffer), "%s%ssb%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
          status = msSaveImage( msObj->Map, img, buffer);
          if(status != MS_SUCCESS) return status;
          msFreeImage(img);
@@ -231,7 +245,7 @@
       {
          img = msDrawReferenceMap(msObj->Map);
          if(!img) return MS_FAILURE;
-         snprintf(buffer, 1024, "%s%sref%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
+         snprintf(buffer, sizeof(buffer), "%s%sref%s.%s", msObj->Map->web.imagepath, msObj->Map->name, msObj->Id, MS_IMAGE_EXTENSION(msObj->Map->outputformat));
          status = msSaveImage(msObj->Map, img, buffer);
          if(status != MS_SUCCESS) return status;
          msFreeImage(img);
@@ -2392,6 +2406,11 @@
           return(NULL);
         }
 
+	if(isValidTemplate(stream, join->header) != MS_TRUE) {
+	  fclose(stream);
+	  return NULL;
+	}
+
         /* echo file to the output buffer, no substitutions */
         while(fgets(line, MS_BUFFER_LENGTH, stream) != NULL) outbuf = msStringConcatenate(outbuf, line);
 
@@ -2402,7 +2421,12 @@
         msSetError(MS_IOERR, "Error while opening join template file %s.", "processOneToManyJoin()", join->template);
         return(NULL);
       }      
-      
+     
+      if(isValidTemplate(stream, join->template) != MS_TRUE) {
+	fclose(stream);
+	return NULL;
+      }
+ 
       records = MS_TRUE;
     }
     
@@ -2417,6 +2441,7 @@
     }
       
     rewind(stream);
+    fgets(line, MS_BUFFER_LENGTH, stream); /* skip the first line since it's the magic string */
   } /* next record */
 
   if(records==MS_TRUE && join->footer) {    
@@ -2425,6 +2450,11 @@
       return(NULL);
     }
 
+    if(isValidTemplate(stream, join->footer) != MS_TRUE) {
+      fclose(stream);
+      return NULL;
+    }
+
     /* echo file to the output buffer, no substitutions */
     while(fgets(line, MS_BUFFER_LENGTH, stream) != NULL) outbuf = msStringConcatenate(outbuf, line);
     

Modified: branches/branch-5-0/mapserver/maptemplate.h
===================================================================
--- branches/branch-5-0/mapserver/maptemplate.h	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/maptemplate.h	2009-07-21 19:34:36 UTC (rev 9199)
@@ -33,7 +33,8 @@
 #include "mapserver.h"
 #include "maphash.h"
 
-#define IDSIZE 128
+#define IDPATTERN "^[0-9A-Za-z]{1,63}$" 
+#define IDSIZE 64 
 #define TEMPLATE_TYPE(s)  (((strncmp("http://", s, 7) == 0) || (strncmp("https://", s, 8) == 0) || (strncmp("ftp://", s, 6)) == 0)  ? MS_URL : MS_FILE)
 #define MAXZOOM 25
 #define MINZOOM -25

Modified: branches/branch-5-0/mapserver/tests/symbols.txt
===================================================================
--- branches/branch-5-0/mapserver/tests/symbols.txt	2009-07-21 17:43:12 UTC (rev 9198)
+++ branches/branch-5-0/mapserver/tests/symbols.txt	2009-07-21 19:34:36 UTC (rev 9199)
@@ -1,22 +1,22 @@
-
-SYMBOL
+SYMBOLSET
+  SYMBOL
     NAME 'circle' 
     TYPE ellipse 
     FILLED true 
     POINTS
       1 1 
     END 
-END
+  END
 
-SYMBOL
+  SYMBOL
     NAME 'xmarks-png'
     TYPE PIXMAP
     IMAGE 'xmarks.png'
-END
+  END
 
-SYMBOL
+  SYMBOL
     NAME 'home-png'
     TYPE PIXMAP
     IMAGE 'home.png'
+  END
 END
-



More information about the mapserver-commits mailing list