Opened 9 years ago

Closed 9 years ago

#3522 closed enhancement (fixed)

mandatory validation patterns

Reported by: regodon Owned by: sdlime
Priority: normal Milestone: 6.0 release
Component: MapServer CGI Version: unspecified
Severity: normal Keywords:
Cc: dmorissette

Description

I find that variable substituion is a powerful but very dangerous feature as it is very prone to sql injections.

I would like that it were mandatory to define validation patterns for each variable (%variable%) present in the mapfile. Or may be there should be a "SECURITY strict" directive to enable/disable this behaviour.

Also, i have found that validation_patterns are global to all the mapfile, not restricted to the layer where they are defined into, so it doesn't matter in which layer you define them. It's logical since variable substituion is global too. So, maybe all validation patterns should be defined together at map level and not inside layers.

Just my thoughts.

Change History (5)

comment:1 Changed 9 years ago by sdlime

Cc: dmorissette added
Milestone: 6.0 release

I tend to agree with you. There's enough rope for users to hang themselves if they're not careful. Personally I'd like to see validation become mandatory and integrated into the VALIDATION ... END block support added in 5.6. That validation is used for URL-based mapfile changes and cascades from class to layer and the to map.

6.0 is the perfect time to change that policy...

Steve

comment:2 Changed 9 years ago by sdlime

Status: newassigned

comment:3 Changed 9 years ago by sdlime

One thing to add. In 5.6 at least the validation strings are layer specific. The docs reflect this...

Steve

comment:4 Changed 9 years ago by regodon

Hi Steve,

It seems the VALIDATION block isn't documented. I didn't know it existed, and all i have found is rfc 44. Is it working and stable?

About _validation_pattern, i've been looking at mapserv.c (5.6.5) and function loadMap does a for loop testing each received paramameter against all layers validation patterns regardless of layer's status. So every _validation_pattern is validated globally.

You can define different validation patterns for the same variable on different layers, but variable will need to validate against all patterns.

Exceprt:

mapObj *loadMap(void)
{
[...]
  /* check for any %variable% substitutions here, also do any map_ changes, we do this here so WMS/WFS  */
  /* services can take advantage of these "vendor specific" extensions */
  for(i=0;i<mapserv->request->NumParams;i++) {

 /* validation pattern metadata key */
    key = (char *)malloc(sizeof(char)*strlen(mapserv->request->ParamNames[i]) + 20);
    sprintf(key,"%s_validation_pattern", mapserv->request->ParamNames[i]);

   for(j=0; j<map->numlayers; j++) {
      layerObj *layer = GET_LAYER(map, j);
      value = msLookupHashTable(&(layer->metadata), key);
      if(value) { /* validate parameter value */
        if(msEvalRegex(value, mapserv->request->ParamValues[i]) == MS_FALSE) {
          msSetError(MS_WEBERR, "Parameter '%s' value fails to validate.", "loadMap()", mapserv->request->ParamNames[i]);
          writeError();
        }
      }
      msLayerSubstituteString(layer, tmpstr, mapserv->request->ParamValues[i]);
    }
[...]
}

comment:5 Changed 9 years ago by sdlime

Resolution: fixed
Status: assignedclosed

Hi all: I've added code in 6.0/trunk to make validation of runtime substitutions mandatory. You can set the pattern via the ..._validation_pattern in layer METADATA (current behavior), in the layer validation block or in the web validation block.

The priority is: layer validation block, layer metadata, web validation block. I'm considering the layer metadata as being deprecated.

I added some tests to msautotest in r11222 that demonstrate the options. The code itself was added in r11226.

I'll open another ticket to make sure the validation block is documented.

Steve

Note: See TracTickets for help on using tickets.