-
-
Notifications
You must be signed in to change notification settings - Fork 76
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[fc] Repository: plone.app.dexterity
Branch: refs/heads/master Date: 2020-11-16T11:44:23+01:00 Author: Maurits van Rees (mauritsvanrees) <[email protected]> Commit: plone/plone.app.dexterity@b11b514 For increased security, in the modeleditor do not resolve entities, and remove processing instructions. See plone/Products.CMFPlone#3209 Files changed: A news/3209.bugfix M plone/app/dexterity/browser/modeleditor.py Repository: plone.app.dexterity Branch: refs/heads/master Date: 2020-11-16T21:17:35+01:00 Author: Maurits van Rees (mauritsvanrees) <[email protected]> Commit: plone/plone.app.dexterity@69b9c31 Merge pull request #317 from plone/maurits/cmfplone-issue-3209-lxml Modeleditor: do not resolve entities, to avoid xml vulnerabilities Files changed: A news/3209.bugfix M plone/app/dexterity/browser/modeleditor.py
- Loading branch information
1 parent
afe3616
commit 7235f09
Showing
1 changed file
with
13 additions
and
14 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,36 @@ | ||
Repository: plone.app.theming | ||
Repository: plone.app.dexterity | ||
|
||
|
||
Branch: refs/heads/master | ||
Date: 2020-11-16T11:18:04+01:00 | ||
Date: 2020-11-16T11:44:23+01:00 | ||
Author: Maurits van Rees (mauritsvanrees) <[email protected]> | ||
Commit: https://github.com/plone/plone.app.theming/commit/b22ccb1b8db526cc426ec6451e162bb5d94481d8 | ||
Commit: https://github.com/plone/plone.app.dexterity/commit/b11b514fb7b79301a5562a6b63150553bf6ece2d | ||
|
||
For increased security, fail when trying file protocol access in diazo rules. | ||
For increased security, in the modeleditor do not resolve entities, and remove processing instructions. | ||
|
||
Also do not resolve entities, and remove processing instructions. | ||
See https://github.com/plone/Products.CMFPlone/issues/3209 | ||
|
||
Files changed: | ||
A news/3209.bugfix | ||
M src/plone/app/theming/utils.py | ||
M plone/app/dexterity/browser/modeleditor.py | ||
|
||
b'diff --git a/news/3209.bugfix b/news/3209.bugfix\nnew file mode 100644\nindex 0000000..fcda3a1\n--- /dev/null\n+++ b/news/3209.bugfix\n@@ -0,0 +1,3 @@\n+For increased security, fail when trying file protocol access in diazo rules.\n+Also do not resolve entities, and remove processing instructions.\n+[maurits]\ndiff --git a/src/plone/app/theming/utils.py b/src/plone/app/theming/utils.py\nindex ed6b734..f2cf3d8 100644\n--- a/src/plone/app/theming/utils.py\n+++ b/src/plone/app/theming/utils.py\n@@ -66,6 +66,16 @@ def theming_policy(request=None):\n return IThemingPolicy(request)\n \n \n+class FailingFileProtocolResolver(etree.Resolver):\n+ """Resolver that fails for security when file:/// urls are tried.\n+ """\n+ def resolve(self, system_url, public_id, context):\n+ if system_url.startswith(\'file://\') and system_url != \'file:///__diazo__\':\n+ # The error will be caught by lxml and we only see this in the traceback:\n+ # XIncludeError: could not load <system_url>, and no fallback was found\n+ raise ValueError("File protocol access not allowed: \'%s\'" % system_url)\n+\n+\n class NetworkResolver(etree.Resolver):\n """Resolver for network urls\n """\n@@ -618,15 +628,16 @@ def getParser(type, readNetwork):\n """\n \n if type == \'rules\':\n- parser = etree.XMLParser(recover=False)\n+ parser = etree.XMLParser(recover=False, resolve_entities=False, remove_pis=True)\n elif type == \'theme\':\n parser = etree.HTMLParser()\n elif type == \'compiler\':\n- parser = etree.XMLParser()\n+ parser = etree.XMLParser(resolve_entities=False, remove_pis=True)\n parser.resolvers.add(InternalResolver())\n parser.resolvers.add(PythonResolver())\n if readNetwork:\n parser.resolvers.add(NetworkResolver())\n+ parser.resolvers.add(FailingFileProtocolResolver())\n return parser\n \n \n' | ||
b"diff --git a/news/3209.bugfix b/news/3209.bugfix\nnew file mode 100644\nindex 0000000..848c253\n--- /dev/null\n+++ b/news/3209.bugfix\n@@ -0,0 +1,2 @@\n+For increased security, in the modeleditor do not resolve entities, and remove processing instructions.\n+[maurits]\ndiff --git a/plone/app/dexterity/browser/modeleditor.py b/plone/app/dexterity/browser/modeleditor.py\nindex 21ecc39..c132a7a 100644\n--- a/plone/app/dexterity/browser/modeleditor.py\n+++ b/plone/app/dexterity/browser/modeleditor.py\n@@ -42,8 +42,12 @@ def __call__(self):\n source = self.request.form.get('source')\n if source:\n # Is it valid XML?\n+ # Some safety measures.\n+ # We do not want to load entities, especially file:/// entities.\n+ # Also discard processing instructions.\n+ parser = etree.XMLParser(resolve_entities=False, remove_pis=True)\n try:\n- root = etree.fromstring(source)\n+ root = etree.fromstring(source, parser=parser)\n except etree.XMLSyntaxError as e:\n return json.dumps({\n 'success': False,\n" | ||
|
||
Repository: plone.app.theming | ||
Repository: plone.app.dexterity | ||
|
||
|
||
Branch: refs/heads/master | ||
Date: 2020-11-16T21:17:18+01:00 | ||
Date: 2020-11-16T21:17:35+01:00 | ||
Author: Maurits van Rees (mauritsvanrees) <[email protected]> | ||
Commit: https://github.com/plone/plone.app.theming/commit/8b23f0d33a961714050abc0ebba99f74ecad6e79 | ||
Commit: https://github.com/plone/plone.app.dexterity/commit/69b9c3121e1740fc0082f3537bf9b564372551f4 | ||
|
||
Merge pull request #193 from plone/maurits/cmfplone-issue-3209-lxml | ||
Merge pull request #317 from plone/maurits/cmfplone-issue-3209-lxml | ||
|
||
Fail when trying file protocol access in diazo rules | ||
Modeleditor: do not resolve entities, to avoid xml vulnerabilities | ||
|
||
Files changed: | ||
A news/3209.bugfix | ||
M src/plone/app/theming/utils.py | ||
M plone/app/dexterity/browser/modeleditor.py | ||
|
||
b'diff --git a/news/3209.bugfix b/news/3209.bugfix\nnew file mode 100644\nindex 0000000..fcda3a1\n--- /dev/null\n+++ b/news/3209.bugfix\n@@ -0,0 +1,3 @@\n+For increased security, fail when trying file protocol access in diazo rules.\n+Also do not resolve entities, and remove processing instructions.\n+[maurits]\ndiff --git a/src/plone/app/theming/utils.py b/src/plone/app/theming/utils.py\nindex ed6b734..f2cf3d8 100644\n--- a/src/plone/app/theming/utils.py\n+++ b/src/plone/app/theming/utils.py\n@@ -66,6 +66,16 @@ def theming_policy(request=None):\n return IThemingPolicy(request)\n \n \n+class FailingFileProtocolResolver(etree.Resolver):\n+ """Resolver that fails for security when file:/// urls are tried.\n+ """\n+ def resolve(self, system_url, public_id, context):\n+ if system_url.startswith(\'file://\') and system_url != \'file:///__diazo__\':\n+ # The error will be caught by lxml and we only see this in the traceback:\n+ # XIncludeError: could not load <system_url>, and no fallback was found\n+ raise ValueError("File protocol access not allowed: \'%s\'" % system_url)\n+\n+\n class NetworkResolver(etree.Resolver):\n """Resolver for network urls\n """\n@@ -618,15 +628,16 @@ def getParser(type, readNetwork):\n """\n \n if type == \'rules\':\n- parser = etree.XMLParser(recover=False)\n+ parser = etree.XMLParser(recover=False, resolve_entities=False, remove_pis=True)\n elif type == \'theme\':\n parser = etree.HTMLParser()\n elif type == \'compiler\':\n- parser = etree.XMLParser()\n+ parser = etree.XMLParser(resolve_entities=False, remove_pis=True)\n parser.resolvers.add(InternalResolver())\n parser.resolvers.add(PythonResolver())\n if readNetwork:\n parser.resolvers.add(NetworkResolver())\n+ parser.resolvers.add(FailingFileProtocolResolver())\n return parser\n \n \n' | ||
b"diff --git a/news/3209.bugfix b/news/3209.bugfix\nnew file mode 100644\nindex 0000000..848c253\n--- /dev/null\n+++ b/news/3209.bugfix\n@@ -0,0 +1,2 @@\n+For increased security, in the modeleditor do not resolve entities, and remove processing instructions.\n+[maurits]\ndiff --git a/plone/app/dexterity/browser/modeleditor.py b/plone/app/dexterity/browser/modeleditor.py\nindex 21ecc39..c132a7a 100644\n--- a/plone/app/dexterity/browser/modeleditor.py\n+++ b/plone/app/dexterity/browser/modeleditor.py\n@@ -42,8 +42,12 @@ def __call__(self):\n source = self.request.form.get('source')\n if source:\n # Is it valid XML?\n+ # Some safety measures.\n+ # We do not want to load entities, especially file:/// entities.\n+ # Also discard processing instructions.\n+ parser = etree.XMLParser(resolve_entities=False, remove_pis=True)\n try:\n- root = etree.fromstring(source)\n+ root = etree.fromstring(source, parser=parser)\n except etree.XMLSyntaxError as e:\n return json.dumps({\n 'success': False,\n" | ||
|