Skip to content

Commit 82a3eb5

Browse files
sdlimerouault
andcommitted
Address flaw in CGI mapfile loading that makes it possible to bypass security controls (#6313) (#6314)
* Create coverity-scan.yml * Update coverity-scan.yml * Avoid resource leak... (CID 1503409) * Revert "Avoid resource leak... (CID 1503409)" This reverts commit 7d261af. * Updated... * Limit action to MapServer/MapServer repo, run every Sunday (for now). * Always force map parameter values through validation checks. Add validation checks on environment variable names. * msIsValidRegex(): fix memleak Co-authored-by: Even Rouault <even.rouault@spatialys.com>
1 parent 3c3c9b3 commit 82a3eb5

File tree

4 files changed

+75
-16
lines changed

4 files changed

+75
-16
lines changed

mapfile.c

+30
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,16 @@ int msValidateParameter(char *value, char *pattern1, char *pattern2, char *patte
104104
return(MS_FAILURE);
105105
}
106106

107+
int msIsValidRegex(const char* e) {
108+
ms_regex_t re;
109+
if(ms_regcomp(&re, e, MS_REG_EXTENDED|MS_REG_NOSUB) != 0) {
110+
msSetError(MS_REGEXERR, "Failed to compile expression (%s).", "msEvalRegex()", e);
111+
return(MS_FALSE);
112+
}
113+
ms_regfree(&re);
114+
return MS_TRUE;
115+
}
116+
107117
int msEvalRegex(const char *e, const char *s)
108118
{
109119
ms_regex_t re;
@@ -124,6 +134,26 @@ int msEvalRegex(const char *e, const char *s)
124134
return(MS_TRUE);
125135
}
126136

137+
int msCaseEvalRegex(const char *e, const char *s)
138+
{
139+
ms_regex_t re;
140+
141+
if(!e || !s) return(MS_FALSE);
142+
143+
if(ms_regcomp(&re, e, MS_REG_EXTENDED|MS_REG_ICASE|MS_REG_NOSUB) != 0) {
144+
msSetError(MS_REGEXERR, "Failed to compile expression (%s).", "msEvalRegex()", e);
145+
return(MS_FALSE);
146+
}
147+
148+
if(ms_regexec(&re, s, 0, NULL, 0) != 0) { /* no match */
149+
ms_regfree(&re);
150+
return(MS_FALSE);
151+
}
152+
ms_regfree(&re);
153+
154+
return(MS_TRUE);
155+
}
156+
127157
#ifdef USE_MSFREE
128158
void msFree(void *p)
129159
{

mapserv.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ int main(int argc, char *argv[])
160160

161161
/* push high-value ENV vars into the CPL global config - primarily for IIS/FastCGI */
162162
const char* const apszEnvVars[] = {
163-
"CURL_CA_BUNDLE", "MS_MAPFILE", "MS_MAP_NO_PATH", "MS_MAP_PATTERN",
163+
"CURL_CA_BUNDLE", "MS_MAPFILE", "MS_MAP_NO_PATH", "MS_MAP_PATTERN", "MS_MAP_ENV_PATTERN",
164+
"MS_MAP_BAD_PATTERN", "MS_MAP_ENV_BAD_PATTERN",
164165
NULL /* guard */ };
165166
for( int i = 0; apszEnvVars[i] != NULL; ++i ) {
166167
const char* value = getenv(apszEnvVars[i]);

mapserver.h

+2
Original file line numberDiff line numberDiff line change
@@ -2122,7 +2122,9 @@ void msPopulateTextSymbolForLabelAndString(textSymbolObj *ts, labelObj *l, char
21222122
MS_DLL_EXPORT char *msWriteReferenceMapToString(referenceMapObj *ref);
21232123
MS_DLL_EXPORT char *msWriteLegendToString(legendObj *legend);
21242124
MS_DLL_EXPORT char *msWriteClusterToString(clusterObj *cluster);
2125+
MS_DLL_EXPORT int msIsValidRegex(const char* e);
21252126
MS_DLL_EXPORT int msEvalRegex(const char *e, const char *s);
2127+
MS_DLL_EXPORT int msCaseEvalRegex(const char *e, const char *s);
21262128
#ifdef USE_MSFREE
21272129
MS_DLL_EXPORT void msFree(void *p);
21282130
#else

mapservutil.c

+41-15
Original file line numberDiff line numberDiff line change
@@ -201,41 +201,67 @@ mapObj *msCGILoadMap(mapservObj *mapserv)
201201
int i, j;
202202
mapObj *map = NULL;
203203

204+
const char *ms_map_bad_pattern_default = "[/\\]{2}|[/\\]?\\.+[/\\]|,";
205+
const char *ms_map_env_bad_pattern_default = "^(AUTH_.*|CERT_.*|CONTENT_(LENGTH|TYPE)|DOCUMENT_(ROOT|URI)|GATEWAY_INTERFACE|HTTP.*|QUERY_STRING|PATH_(INFO|TRANSLATED)|REMOTE_.*|REQUEST_(METHOD|URI)|SCRIPT_(FILENAME|NAME)|SERVER_.*)";
206+
207+
int ms_mapfile_tainted = MS_TRUE;
204208
const char *ms_mapfile = CPLGetConfigOption("MS_MAPFILE", NULL);
209+
205210
const char *ms_map_no_path = CPLGetConfigOption("MS_MAP_NO_PATH", NULL);
206211
const char *ms_map_pattern = CPLGetConfigOption("MS_MAP_PATTERN", NULL);
212+
const char *ms_map_env_pattern = CPLGetConfigOption("MS_MAP_ENV_PATTERN", NULL);
213+
214+
const char *ms_map_bad_pattern = CPLGetConfigOption("MS_MAP_BAD_PATTERN", NULL);
215+
if(ms_map_bad_pattern == NULL) ms_map_bad_pattern = ms_map_bad_pattern_default;
216+
217+
const char *ms_map_env_bad_pattern = CPLGetConfigOption("MS_MAP_ENV_BAD_PATTERN", NULL);
218+
if(ms_map_env_bad_pattern == NULL) ms_map_env_bad_pattern = ms_map_env_bad_pattern_default;
207219

208220
for(i=0; i<mapserv->request->NumParams; i++) /* find the mapfile parameter first */
209221
if(strcasecmp(mapserv->request->ParamNames[i], "map") == 0) break;
210222

211223
if(i == mapserv->request->NumParams) {
212-
if(ms_mapfile != NULL) {
213-
map = msLoadMap(ms_mapfile,NULL);
214-
} else {
224+
if(ms_mapfile == NULL) {
215225
msSetError(MS_WEBERR, "CGI variable \"map\" is not set.", "msCGILoadMap()"); /* no default, outta here */
216226
return NULL;
217227
}
228+
ms_mapfile_tainted = MS_FALSE;
218229
} else {
219-
if(getenv(mapserv->request->ParamValues[i])) /* an environment variable references the actual file to use */
220-
map = msLoadMap(getenv(mapserv->request->ParamValues[i]), NULL);
221-
else {
222-
/* by here we know the request isn't for something in an environment variable */
223-
if(ms_map_no_path != NULL) {
224-
msSetError(MS_WEBERR, "Mapfile not found in environment variables and this server is not configured for full paths.", "msCGILoadMap()");
230+
if(getenv(mapserv->request->ParamValues[i])) { /* an environment variable references the actual file to use */
231+
/* validate env variable name */
232+
if(msIsValidRegex(ms_map_env_bad_pattern) == MS_FALSE || msCaseEvalRegex(ms_map_env_bad_pattern, mapserv->request->ParamValues[i]) == MS_TRUE) {
233+
msSetError(MS_WEBERR, "CGI variable \"map\" fails to validate.", "msCGILoadMap()");
225234
return NULL;
226235
}
227-
228-
if(ms_map_pattern != NULL && msEvalRegex(ms_map_pattern, mapserv->request->ParamValues[i]) != MS_TRUE) {
229-
msSetError(MS_WEBERR, "Parameter 'map' value fails to validate.", "msCGILoadMap()");
236+
if(ms_map_env_pattern != NULL && msEvalRegex(ms_map_env_pattern, mapserv->request->ParamValues[i]) != MS_TRUE) {
237+
msSetError(MS_WEBERR, "CGI variable \"map\" fails to validate.", "msCGILoadMap()");
230238
return NULL;
231239
}
240+
ms_mapfile = getenv(mapserv->request->ParamValues[i]);
241+
} else {
242+
/* by now we know the request isn't for something in an environment variable */
243+
if(ms_map_no_path != NULL) {
244+
msSetError(MS_WEBERR, "CGI variable \"map\" not found in environment and this server is not configured for full paths.", "msCGILoadMap()");
245+
return NULL;
246+
}
247+
ms_mapfile = mapserv->request->ParamValues[i];
248+
}
249+
}
232250

233-
/* ok to try to load now */
234-
map = msLoadMap(mapserv->request->ParamValues[i], NULL);
251+
/* validate ms_mapfile if tainted */
252+
if(ms_mapfile_tainted == MS_TRUE) {
253+
if(msIsValidRegex(ms_map_bad_pattern) == MS_FALSE || msEvalRegex(ms_map_bad_pattern, ms_mapfile) == MS_TRUE) {
254+
msSetError(MS_WEBERR, "CGI variable \"map\" fails to validate.", "msCGILoadMap()");
255+
return NULL;
256+
}
257+
if(ms_map_pattern != NULL && msEvalRegex(ms_map_pattern, ms_mapfile) != MS_TRUE) {
258+
msSetError(MS_WEBERR, "CGI variable \"map\" fails to validate.", "msCGILoadMap()");
259+
return NULL;
235260
}
236261
}
237-
238262

263+
/* ok to try to load now */
264+
map = msLoadMap(ms_mapfile, NULL);
239265
if(!map) return NULL;
240266

241267
if(!msLookupHashTable(&(map->web.validation), "immutable")) {

0 commit comments

Comments
 (0)