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

Base logic for CVE-2023-42458 on the media type proper #1167

Merged
merged 3 commits into from
Sep 27, 2023

Conversation

d-maurer
Copy link
Contributor

@d-maurer d-maurer commented Sep 27, 2023

ignore parameters and whitespace and normalize to lowercase.

Motivated by the report from Almir Zunic on "mailto:security@plone.org" " [p/z-sec] XSS - found in Management interface" I reexamined the logic developed for CVE-2023-42458 and found it incomplete. It compares the whole content_type against a list of types. But, at least if set manually, content_type can contain parameters and/or additional whitespace and may use uppercase letters which could not be directly compared with a types list.

When the logic is based on an allow list (the default), files with an unexpected content_type are delivered as attachment (which is safe). However, if the logic is based on a deny list, potentially dangerous files could be presented inline.

This PR bases the CVE-2023-42458 logic on the media type proper: it ignores parameters and whitespace and normalizes to lowercase.

@d-maurer d-maurer marked this pull request as ready for review September 27, 2023 06:03
Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

Yes, this works for me when I test it in Plone 6.0. Thanks!

@d-maurer d-maurer enabled auto-merge (squash) September 27, 2023 09:28
@d-maurer d-maurer merged commit 9b52f66 into master Sep 27, 2023
@d-maurer d-maurer deleted the extract_media_type branch September 27, 2023 09:31
@d-maurer
Copy link
Contributor Author

Thank you for the reviews!

mauritsvanrees added a commit to plone/plone.namedfile that referenced this pull request Oct 27, 2023
mauritsvanrees added a commit to plone/plone.restapi that referenced this pull request Oct 27, 2023
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Oct 28, 2023
Branch: refs/heads/master
Date: 2023-10-27T02:20:31+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@6687472

Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167

Files changed:
A news/1167.bugfix
M plone/namedfile/browser.py
M plone/namedfile/scaling.py
M plone/namedfile/utils/__init__.py
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2023-10-27T09:32:21+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@17dbec4

Only define extract_media_type when we cannot import it from Zope.

Files changed:
M plone/namedfile/utils/__init__.py
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2023-10-27T09:41:23+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@ff616cc

Copy unit test for extract_media_type from Zope.

Files changed:
M plone/namedfile/tests/test_image.py
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2023-10-28T10:00:32-07:00
Author: David Glick (davisagli) <david@glicksoftware.com>
Commit: plone/plone.namedfile@17a21d4

Merge pull request #154 from plone/maurits-extract-media-type-master

Be more strict when checking if mimetype is allowed inline

Files changed:
A news/1167.bugfix
M plone/namedfile/browser.py
M plone/namedfile/scaling.py
M plone/namedfile/tests/test_image.py
M plone/namedfile/utils/__init__.py
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Oct 28, 2023
Branch: refs/heads/master
Date: 2023-10-27T02:20:31+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@6687472

Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167

Files changed:
A news/1167.bugfix
M plone/namedfile/browser.py
M plone/namedfile/scaling.py
M plone/namedfile/utils/__init__.py
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2023-10-27T09:32:21+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@17dbec4

Only define extract_media_type when we cannot import it from Zope.

Files changed:
M plone/namedfile/utils/__init__.py
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2023-10-27T09:41:23+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@ff616cc

Copy unit test for extract_media_type from Zope.

Files changed:
M plone/namedfile/tests/test_image.py
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2023-10-28T10:00:32-07:00
Author: David Glick (davisagli) <david@glicksoftware.com>
Commit: plone/plone.namedfile@17a21d4

Merge pull request #154 from plone/maurits-extract-media-type-master

Be more strict when checking if mimetype is allowed inline

Files changed:
A news/1167.bugfix
M plone/namedfile/browser.py
M plone/namedfile/scaling.py
M plone/namedfile/tests/test_image.py
M plone/namedfile/utils/__init__.py
tisto pushed a commit to plone/plone.restapi that referenced this pull request Oct 28, 2023
* Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167.
See also plone/plone.namedfile#154

* Copy a test for extract_media_type from Zope.

* Rename extract_media_type to _extract_media_type to signal it as private.
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Oct 28, 2023
Branch: refs/heads/main
Date: 2023-10-28T22:15:39+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.restapi@f37cd9c

Be more strict when checking if mimetype is allowed inline. (#1724)

* Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167.
See also plone/plone.namedfile#154

* Copy a test for extract_media_type from Zope.

* Rename extract_media_type to _extract_media_type to signal it as private.

Files changed:
A news/1167.bugfix
M src/plone/restapi/services/users/get.py
M src/plone/restapi/tests/test_services_users.py
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Oct 28, 2023
Branch: refs/heads/main
Date: 2023-10-28T22:15:39+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.restapi@f37cd9c

Be more strict when checking if mimetype is allowed inline. (#1724)

* Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167.
See also plone/plone.namedfile#154

* Copy a test for extract_media_type from Zope.

* Rename extract_media_type to _extract_media_type to signal it as private.

Files changed:
A news/1167.bugfix
M src/plone/restapi/services/users/get.py
M src/plone/restapi/tests/test_services_users.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants