Verify/enhance security for runtime substitutions...

[sdlime]

Man, I swear I already added this bug. Anyway, basically I think we need to 
make sure that runtime substitutions are not a big security hole. We only 
support a few but allowing unfiltered substitution is likely dangerous. There 
is no way to say only integers allowed. While I don't believe there has ever 
been an exploit there is the potential for buffer overruns (less likely) or SQL 
injection when substituting into a WHERE clause for one of the databases.

I don't know that there is one set of filtering that we could implement that 
would work in all cases. Rather I think we need to allow for adhoc filtering 
via regular expressions.

One idea might be to allow users to define filters based on variable names in a 
layer metadata section. For example, let's say you have an expression like:

  EXPRESSION ('[ID]' eq '%myid%')

so 'myid' is being passed into MapServer. Then you might set metadata like:

  METADATA
    myid_pattern  '[0-9]{5,10}?'
    ...
  END

Which would restrict the value of 'myid' to a number between 5 and 10 
characters long.

Implementation would be pretty simple, impacting only a small portion of 
mapserv.c

Steve

[sdlime]

I committed an enhancement to mapserv.c today that allows users to define a
validation pattern in layer metadata. For example, let's say you are use a
variable foo to pass in a runtime change to an expression. Then you could set
something like:

  foo_validation_pattern "\d{3}"

The value supplied for foo *must* be a 3 digit integer. If not a validation
error is returned.

Seems to work fine in testing. I chose layer-level metadata since all
substitutions are at the layer-level.

There are a couple of downsides:

  - users are not required to set a validation pattern
  - regex isn't the easiest of languages to master

Thoughts?

Steve

[sdlime]

With no feedback I'll assume the changes are ok. This doesn't break
compatability and while technically a new feature I think anything security
related should be backported to the current version (which I have done).
Changing to a documentation bug...

Steve

[dmorissette]

Has this been documented already?

[hobu]

I think http://mapserver.org/cgi/runsub.html has it fairly well now. Done quite a while ago, so I'll move the milestone.