[mapserver-commits] r8823 - branches/branch-4-10/mapserver

svn at osgeo.org svn at osgeo.org
Wed Mar 25 20:55:05 EDT 2009


Author: sdlime
Date: 2009-03-25 20:55:04 -0400 (Wed, 25 Mar 2009)
New Revision: 8823

Modified:
   branches/branch-4-10/mapserver/HISTORY.TXT
   branches/branch-4-10/mapserver/cgiutil.c
   branches/branch-4-10/mapserver/mapfile.c
   branches/branch-4-10/mapserver/mapquery.c
   branches/branch-4-10/mapserver/mapserv.c
   branches/branch-4-10/mapserver/mapsymbol.c
   branches/branch-4-10/mapserver/maptemplate.c
   branches/branch-4-10/mapserver/maptemplate.h
Log:
Fixed several security issues found in an audit of the CGI application (#2939, #2941, #2942, #2943, #2944)

Modified: branches/branch-4-10/mapserver/HISTORY.TXT
===================================================================
--- branches/branch-4-10/mapserver/HISTORY.TXT	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/HISTORY.TXT	2009-03-26 00:55:04 UTC (rev 8823)
@@ -13,6 +13,8 @@
 Current Version (SVN branch-4-10):
 ---------------------------------
 
+- Fixed several security issues found in an audit of the CGI application (#2939, #2941, #2942, #2943, #2944)
+
 - Fixed java and csharp Makefile.in to use -lmapserver instead of -lmap
   (side-effect of #2150)
 

Modified: branches/branch-4-10/mapserver/cgiutil.c
===================================================================
--- branches/branch-4-10/mapserver/cgiutil.c	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/cgiutil.c	2009-03-26 00:55:04 UTC (rev 8823)
@@ -69,7 +69,8 @@
 static char *readPostBody( cgiRequestObj *request ) 
 {
     char *data; 
-    int data_max, data_len, chunk_size;
+    unsigned int data_max, data_len;
+    int chunk_size;
 
     msIO_needBinaryStdin();
     
@@ -84,7 +85,7 @@
         if( data == NULL )
         {
             msIO_printf("Content-type: text/html%c%c",10,10);
-            msIO_printf("malloc() failed, Content-Length: %d unreasonably large?\n",
+            msIO_printf("malloc() failed, Content-Length: %u unreasonably large?\n",
                    data_max );
             exit( 1 );
         }
@@ -118,7 +119,7 @@
             if( data == NULL )
             {
                 msIO_printf("Content-type: text/html%c%c",10,10);
-                msIO_printf("out of memory trying to allocate %d input buffer, POST body too large?\n", data_max+1 );
+                msIO_printf("out of memory trying to allocate %u input buffer, POST body too large?\n", data_max+1 );
                 exit(1);
             }
         }

Modified: branches/branch-4-10/mapserver/mapfile.c
===================================================================
--- branches/branch-4-10/mapserver/mapfile.c	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/mapfile.c	2009-03-26 00:55:04 UTC (rev 8823)
@@ -4563,6 +4563,9 @@
   int i,j,k;
   char szPath[MS_MAXPATHLEN], szCWDPath[MS_MAXPATHLEN];
 
+  int foundMapToken=MS_FALSE;
+  int token;
+
   if(!filename) {
     msSetError(MS_MISCERR, "Filename is undefined.", "msLoadMap()");
     return(NULL);
@@ -4614,8 +4617,15 @@
 
   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(NULL);
+    }
 
+    switch(token) {
+
     case(CONFIG):
     {
         char *key=NULL, *value=NULL;
@@ -4739,7 +4749,8 @@
       if(loadLegend(&(map->legend), map) == -1) return(NULL);
       break;
     case(MAP):
-      break;   
+      foundMapToken = MS_TRUE;
+      break;
     case(MAXSIZE):
       if(getInteger(&(map->maxsize)) == -1) return(NULL);
       break;

Modified: branches/branch-4-10/mapserver/mapquery.c
===================================================================
--- branches/branch-4-10/mapserver/mapquery.c	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/mapquery.c	2009-03-26 00:55:04 UTC (rev 8823)
@@ -153,6 +153,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-4-10/mapserver/mapserv.c
===================================================================
--- branches/branch-4-10/mapserver/mapserv.c	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/mapserv.c	2009-03-26 00:55:04 UTC (rev 8823)
@@ -438,6 +438,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;
     }
@@ -1261,7 +1265,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();
     }
 
@@ -1618,7 +1622,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-4-10/mapserver/mapsymbol.c
===================================================================
--- branches/branch-4-10/mapserver/mapsymbol.c	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/mapsymbol.c	2009-03-26 00:55:04 UTC (rev 8823)
@@ -635,7 +635,7 @@
 int msLoadSymbolSet(symbolSetObj *symbolset, mapObj *map)
 {
     int retval = MS_FAILURE;
-    
+
     msAcquireLock( TLOCK_PARSER );
     retval = loadSymbolSet( symbolset, map );
     msReleaseLock( TLOCK_PARSER );
@@ -650,6 +650,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);
@@ -676,7 +679,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;
@@ -691,6 +702,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-4-10/mapserver/maptemplate.c
===================================================================
--- branches/branch-4-10/mapserver/maptemplate.c	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/maptemplate.c	2009-03-26 00:55:04 UTC (rev 8823)
@@ -299,7 +299,7 @@
       img = msDrawQueryMap(msObj->Map);
       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;
@@ -310,7 +310,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);
@@ -320,7 +320,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);
@@ -330,7 +330,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);
@@ -3422,7 +3422,7 @@
         image = msDrawMap(msObj->Map);
 
       if(image) { 
-        sprintf(buffer, "%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));
 
         if(msSaveImage(msObj->Map, image, buffer) != MS_SUCCESS && bReturnOnError) {
           msFreeImage(image);
@@ -3440,7 +3440,7 @@
       imageObj *image = NULL;
       image = msDrawLegend(msObj->Map, MS_FALSE);
       if(image) { 
-        sprintf(buffer, "%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));
                 
         if(msSaveImage(msObj->Map, image, buffer) != MS_SUCCESS && bReturnOnError) {
           msFreeImage(image);
@@ -3458,7 +3458,7 @@
       imageObj *image = NULL;
       image = msDrawScalebar(msObj->Map);
       if(image) {
-        sprintf(buffer, "%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));
         if(msSaveImage(msObj->Map, image, buffer) != MS_SUCCESS && bReturnOnError) {
           msFreeImage(image);
           return MS_FALSE;
@@ -3475,7 +3475,7 @@
       imageObj *image;
       image = msDrawReferenceMap(msObj->Map);
       if(image) { 
-        sprintf(buffer, "%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));
         if(msSaveImage(msObj->Map, image, buffer) != MS_SUCCESS && bReturnOnError) {
           msFreeImage(image);
           return MS_FALSE;

Modified: branches/branch-4-10/mapserver/maptemplate.h
===================================================================
--- branches/branch-4-10/mapserver/maptemplate.h	2009-03-26 00:47:01 UTC (rev 8822)
+++ branches/branch-4-10/mapserver/maptemplate.h	2009-03-26 00:55:04 UTC (rev 8823)
@@ -45,7 +45,8 @@
 #include "map.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



More information about the mapserver-commits mailing list