Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address flaw in CGI mapfile loading that makes it possible to bypass security controls (#6313) #6314

Merged
merged 14 commits into from Apr 30, 2021

Conversation

sdlime
Copy link
Member

@sdlime sdlime commented Apr 30, 2021

This pull request addresses a critical flaw identified in issue #6313. The fix forces the mapfile value through validation if it's pulled from an environment variable or given as a path. --Steve

Copy link
Contributor

@rouault rouault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just added 4d631fc to fix a memory leak which was very likely what the ASAN config of Travis-CI didn't like. Hopefully that will make it happy

@sdlime
Copy link
Member Author

sdlime commented Apr 30, 2021

The pull request does a few other things:

  • adds validation to exclude standard CGI environment variables as map values
  • adds ability to add additional user-defined filtering against environment variable values (MS_MAP_ENV_PATTERN)
  • simplifies MS_MAP_PATTERN writing by excluding problematic character sets (e.g. ..) from map values by default.

What does this mean for users? This represents a critical flaw and all users should upgrade ASAP.

If you've not used MS_MAP_PATTERN or MS_MAP_NO_PATH as part of securing your installation then this doesn't have much impact. That said, you should upgrade and make use of those controls to limit where mapfiles can be accessed.

If you've relied on MS_MAP_PATTERN exclusively, you should upgrade and should be in good shape. However, it's a great time to review and test MS_MAP_PATTERN.

If you've relied on MS_MAP_NO_PATH primarily (like me), you should upgrade and set a value for MS_MAP_PATTERN. You might consider also setting a value for MS_MAP_ENV_PATTERN if you have a standard way of naming environment variables that reference mapfiles (for example, mine all end in _MAPFILE).

--Steve

@sdlime sdlime added the backport branch-7-6 To backport a pull request to branch-7-6 label Apr 30, 2021
@sdlime
Copy link
Member Author

sdlime commented Apr 30, 2021

I've just added 4d631fc to fix a memory leak which was very likely what the ASAN config of Travis-CI didn't like. Hopefully that will make it happy

Thanks @rouault! Once the tests pass then I can merge and the backport to 7.6 can happen. Hopefully can do a couple of cherry-picks for 7.0 to 7.4 branches.

@sdlime sdlime merged commit c9542df into MapServer:main Apr 30, 2021
@sdlime sdlime deleted the issue-6313 branch April 30, 2021 20:04
rouault added a commit to rouault/mapserver that referenced this pull request Apr 30, 2021
…security controls (MapServer#6313) (MapServer#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>
sdlime added a commit that referenced this pull request Apr 30, 2021
…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>
sdlime added a commit that referenced this pull request Apr 30, 2021
…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>
sdlime added a commit that referenced this pull request Apr 30, 2021
…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>
rouault added a commit that referenced this pull request Apr 30, 2021
…security controls (#6313) (#6314) (#6315)

Co-authored-by: Even Rouault <even.rouault@spatialys.com>

Co-authored-by: Steve Lime <steve.lime@state.mn.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport branch-7-6 To backport a pull request to branch-7-6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants