Permalink
Browse files

fix denials of service in msApplySLD() (#4703)

Avoid O(N^2) looping on supplied NamedLayers
Only allow HTTP and HTTPS for curl request
unlink temporary sld file in case of error
limit remote SLD downloads to 1 Megabyte by default, can be overriden by the
  (wms|ows)_remote_sld_max_bytes metadata entry
  • Loading branch information...
1 parent 578424b commit 0294d6afe36d6bf1476ae31ccb8d79ffa04184ba @tbonfort tbonfort committed Jul 27, 2013
Showing with 60 additions and 23 deletions.
  1. +12 −1 CMakeLists.txt
  2. +1 −1 mapcontext.c
  3. +13 −1 maphttp.c
  4. +2 −1 maphttp.h
  5. +29 −16 mapogcsld.c
  6. +1 −1 mapsymbol.c
  7. +1 −1 msautotest
  8. +1 −1 run-test-suite.sh
View
@@ -126,6 +126,7 @@ option(WITH_GDAL "Choose if GDAL input raster support should be built in" ON)
option(WITH_OGR "Choose if OGR/GDAL input vector support should be built in" ON)
option(WITH_CLIENT_WMS "Enable Client WMS Layer support (requires CURL and GDAL support)" OFF)
option(WITH_CLIENT_WFS "Enable Client WMS Layer support (requires CURL and OGR support)" OFF)
+option(WITH_CURL "Enable Curl HTTP support (required for wms/wfs client, and remote SLD)" OFF)
option(WITH_WFS "Enable WFS Server support (requires PROJ and OGR support)" ON)
option(WITH_WCS "Enable WCS Server support (requires PROJ and GDAL support)" ON)
option(WITH_LIBXML2 "Choose if libxml2 support should be built in (used for sos, wcs 1.1,2.0 and wfs 1.1)" ON)
@@ -543,19 +544,29 @@ if(WITH_OGR)
endif(WITH_OGR)
if((WITH_CLIENT_WMS) OR (WITH_CLIENT_WFS))
+ set(WITH_CURL ON)
+endif((WITH_CLIENT_WMS) OR (WITH_CLIENT_WFS))
+
+if(WITH_CURL)
find_package(CURL)
if(CURL_FOUND)
include_directories(${CURL_INCLUDE_DIR})
ms_link_libraries( ${CURL_LIBRARY})
set(USE_CURL 1)
else(CURL_FOUND)
+ report_optional_not_found(CURL)
+ endif(CURL_FOUND)
+endif(WITH_CURL)
+
+if((WITH_CLIENT_WMS) OR (WITH_CLIENT_WFS))
+ if(NOT USE_CURL)
if(WITH_CLIENT_WFS)
report_dependency_error(CLIENT_WFS CURL)
endif(WITH_CLIENT_WFS)
if(WITH_CLIENT_WMS)
report_dependency_error(CLIENT_WMS CURL)
endif(WITH_CLIENT_WMS)
- endif(CURL_FOUND)
+ endif(NOT USE_CURL)
endif((WITH_CLIENT_WMS) OR (WITH_CLIENT_WFS))
if(WITH_CLIENT_WMS)
View
@@ -1067,7 +1067,7 @@ int msLoadMapContextURL(mapObj *map, char *urlfilename, int unique_layer_names)
}
pszTmpFile = msTmpFile(map, map->mappath, NULL, "context.xml");
- if (msHTTPGetFile(urlfilename, pszTmpFile, &status,-1, 0, 0) == MS_SUCCESS) {
+ if (msHTTPGetFile(urlfilename, pszTmpFile, &status,-1, 0, 0, 0) == MS_SUCCESS) {
return msLoadMapContext(map, pszTmpFile, unique_layer_names);
} else {
msSetError(MS_MAPCONTEXTERR,
View
@@ -130,6 +130,7 @@ void msHTTPInitRequestObj(httpRequestObj *pasReqInfo, int numRequests)
pasReqInfo[i].pszOutputFile = NULL;
pasReqInfo[i].nLayerId = 0;
pasReqInfo[i].nTimeout = 0;
+ pasReqInfo[i].nMaxBytes = 0;
pasReqInfo[i].nStatus = 0;
pasReqInfo[i].pszContentType = NULL;
pasReqInfo[i].pszErrBuf = NULL;
@@ -224,8 +225,15 @@ static size_t msHTTPWriteFct(void *buffer, size_t size, size_t nmemb,
psReq->nLayerId, (int)(size*nmemb));
}
+ if(psReq->nMaxBytes > 0 && (psReq->result_size + size*nmemb) > psReq->nMaxBytes) {
+ msSetError(MS_HTTPERR, "Requested transfer larger than configured maximum %d.",
+ "msHTTPWriteFct()",
+ psReq->nMaxBytes );
+ return -1;
+ }
/* Case where we are writing to a disk file. */
if( psReq->fp != NULL ) {
+ psReq->result_size += size*nmemb;
return fwrite(buffer, size, nmemb, psReq->fp);
}
@@ -529,6 +537,8 @@ int msHTTPExecuteRequests(httpRequestObj *pasReqInfo, int numRequests,
/* set URL, note that curl keeps only a ref to our string buffer */
curl_easy_setopt(http_handle, CURLOPT_URL, pasReqInfo[i].pszGetUrl );
+ curl_easy_setopt(http_handle, CURLOPT_PROTOCOLS, CURLPROTO_HTTP|CURLPROTO_HTTPS );
+
/* Set User-Agent (auto-generate if not set by caller */
if (pasReqInfo[i].pszUserAgent == NULL) {
curl_version_info_data *psCurlVInfo;
@@ -914,7 +924,7 @@ int msHTTPExecuteRequests(httpRequestObj *pasReqInfo, int numRequests,
**********************************************************************/
int msHTTPGetFile(const char *pszGetUrl, const char *pszOutputFile,
int *pnHTTPStatus, int nTimeout, int bCheckLocalCache,
- int bDebug)
+ int bDebug, int nMaxBytes)
{
httpRequestObj *pasReqInfo;
@@ -931,6 +941,8 @@ int msHTTPGetFile(const char *pszGetUrl, const char *pszOutputFile,
pasReqInfo[0].pszGetUrl = msStrdup(pszGetUrl);
pasReqInfo[0].pszOutputFile = msStrdup(pszOutputFile);
pasReqInfo[0].debug = (char)bDebug;
+ pasReqInfo[0].nTimeout = nTimeout;
+ pasReqInfo[0].nMaxBytes = nMaxBytes;
if (msHTTPExecuteRequests(pasReqInfo, 1, bCheckLocalCache) != MS_SUCCESS) {
*pnHTTPStatus = pasReqInfo[0].nStatus;
View
@@ -60,6 +60,7 @@ extern "C" {
char *pszGetUrl;
char *pszOutputFile;
int nTimeout;
+ int nMaxBytes;
rectObj bbox;
int width;
int height;
@@ -106,7 +107,7 @@ extern "C" {
int bCheckLocalCache);
int msHTTPGetFile(const char *pszGetUrl, const char *pszOutputFile,
int *pnHTTPStatus, int nTimeout, int bCheckLocalCache,
- int bDebug);
+ int bDebug, int nMaxBytes);
int msHTTPAuthProxySetup(hashTableObj *mapmd, hashTableObj *lyrmd,
httpRequestObj *pasReqInfo, int numRequests,
View
@@ -77,23 +77,35 @@ int msSLDApplySLDURL(mapObj *map, char *szURL, int iLayer,
if (pszSLDTmpFile == NULL) {
pszSLDTmpFile = msTmpFile(map, NULL, NULL, "sld.xml" );
}
- if (msHTTPGetFile(szURL, pszSLDTmpFile, &status,-1, 0, 0) == MS_SUCCESS) {
- if ((fp = fopen(pszSLDTmpFile, "rb")) != NULL) {
- int nBufsize=0;
- fseek(fp, 0, SEEK_END);
- nBufsize = ftell(fp);
- rewind(fp);
- pszSLDbuf = (char*)malloc((nBufsize+1)*sizeof(char));
- fread(pszSLDbuf, 1, nBufsize, fp);
- fclose(fp);
- pszSLDbuf[nBufsize] = '\0';
+ if (pszSLDTmpFile == NULL) {
+ msSetError(MS_WMSERR, "Could not determine temporary file %s. Please make sure that the temporary path is set. The temporary path can be defined for example by setting TMPPATH in the map file. Please check the MapServer documentation on temporary path settings.", "msSLDApplySLDURL()", pszSLDTmpFile);
+ } else {
+ int nMaxRemoteSLDBytes;
+ const char *pszMaxRemoteSLDBytes = msOWSLookupMetadata(&(map->web.metadata), "MO", "remote_sld_max_bytes");
+ if(!pszMaxRemoteSLDBytes) {
+ nMaxRemoteSLDBytes = 1024*1024; /* 1 megaByte */
+ } else {
+ nMaxRemoteSLDBytes = atoi(pszMaxRemoteSLDBytes);
+ }
+ if (msHTTPGetFile(szURL, pszSLDTmpFile, &status,-1, 0, 0, nMaxRemoteSLDBytes) == MS_SUCCESS) {
+ if ((fp = fopen(pszSLDTmpFile, "rb")) != NULL) {
+ int nBufsize=0;
+ fseek(fp, 0, SEEK_END);
+ nBufsize = ftell(fp);
+ rewind(fp);
+ pszSLDbuf = (char*)malloc((nBufsize+1)*sizeof(char));
+ fread(pszSLDbuf, 1, nBufsize, fp);
+ fclose(fp);
+ pszSLDbuf[nBufsize] = '\0';
+ unlink(pszSLDTmpFile);
+ }
+ } else {
unlink(pszSLDTmpFile);
+ msSetError(MS_WMSERR, "Could not open SLD %s and save it in a temporary file. Please make sure that the sld url is valid and that the temporary path is set. The temporary path can be defined for example by setting TMPPATH in the map file. Please check the MapServer documentation on temporary path settings.", "msSLDApplySLDURL", szURL);
}
- } else {
- msSetError(MS_WMSERR, "Could not open SLD %s and save it in a temporary file. Please make sure that the sld url is valid and that the temporary path is set. The temporary path can be defined for example by setting TMPPATH in the map file. Please check the MapServer documentation on temporary path settings.", "msSLDApplySLDURL", szURL);
+ if (pszSLDbuf)
+ nStatus = msSLDApplySLD(map, pszSLDbuf, iLayer, pszStyleLayerName, ppszLayerNames);
}
- if (pszSLDbuf)
- nStatus = msSLDApplySLD(map, pszSLDbuf, iLayer, pszStyleLayerName, ppszLayerNames);
}
return nStatus;
@@ -161,11 +173,12 @@ int msSLDApplySLD(mapObj *map, char *psSLDXML, int iLayer,
layerObj *psTmpLayer=NULL;
int nIndex;
char tmpId[128];
+ nIndex = msGetLayerIndex(map, pasLayers[m].name);
+ if(pasLayers[m].name == NULL) continue;
for (l=0; l<nLayers; l++) {
- if(pasLayers[m].name == NULL || pasLayers[l].name == NULL)
+ if(pasLayers[l].name == NULL)
continue;
- nIndex = msGetLayerIndex(map, pasLayers[m].name);
if (m !=l && strcasecmp(pasLayers[m].name, pasLayers[l].name)== 0 &&
nIndex != -1) {
View
@@ -375,7 +375,7 @@ int msAddImageSymbol(symbolSetObj *symbolset, char *filename)
tmpfullfilename = msBuildPath(szPath, tmppath, tmpfilename);
if (tmpfullfilename) {
/*use the url for now as a caching mechanism*/
- if (msHTTPGetFile(filename, tmpfullfilename, &status, -1, bCheckLocalCache, 0) == MS_SUCCESS) {
+ if (msHTTPGetFile(filename, tmpfullfilename, &status, -1, bCheckLocalCache, 0, 1024*1024 /* 1 MegaByte */) == MS_SUCCESS) {
symbol->imagepath = msStrdup(tmpfullfilename);
symbol->full_pixmap_path = msStrdup(tmpfullfilename);
}
View
@@ -18,7 +18,7 @@ if [ -f CMakeLists.txt ]; then
fi
mkdir -p build
cd build
- cmake .. -DCMAKE_C_FLAGS="--coverage" -DCMAKE_CXX_FLAGS="--coverage" -DCMAKE_SHARED_LINKER_FLAGS="-lgcov" -DWITH_GD=1 -DWITH_CLIENT_WMS=1 -DWITH_CLIENT_WFS=1 -DWITH_KML=1 -DWITH_SOS=1 -DWITH_PHP=1 -DWITH_PYTHON=1 -DWITH_FRIBIDI=0 -DWITH_FCGI=0 -DWITH_EXEMPI=1 -DCMAKE_BUILD_TYPE=Release -DWITH_RSVG=1
+ cmake .. -DCMAKE_C_FLAGS="--coverage" -DCMAKE_CXX_FLAGS="--coverage" -DCMAKE_SHARED_LINKER_FLAGS="-lgcov" -DWITH_GD=1 -DWITH_CLIENT_WMS=1 -DWITH_CLIENT_WFS=1 -DWITH_KML=1 -DWITH_SOS=1 -DWITH_PHP=1 -DWITH_PYTHON=1 -DWITH_FRIBIDI=0 -DWITH_FCGI=0 -DWITH_EXEMPI=1 -DCMAKE_BUILD_TYPE=Release -DWITH_RSVG=1 -DWITH_CURL=1
make -j4
cd ..
make -f Makefile.autotest -j4 test

0 comments on commit 0294d6a

Please sign in to comment.