diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 1254884a..5c7a28cd 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -29,7 +29,7 @@ jobs: if [ -f requirements.txt ]; then pip install -r requirements.txt; fi if [ -f test_requirements.txt ]; then pip install -r test_requirements.txt; fi - name: Install wkhtmltopdf - run: sudo apt install -y wkhtmltopdf + run: sudo apt-get update && sudo apt install -y wkhtmltopdf - name: Lint with flake8 run: | # stop the build if there are Python syntax errors or undefined names diff --git a/.gitignore b/.gitignore index 8145dcc5..e500c6bf 100644 --- a/.gitignore +++ b/.gitignore @@ -46,6 +46,7 @@ __pycache__/ !/example-msg-files/multi-to.msg !/example-msg-files/multi-to-to.msg !/example-msg-files/unicode-header.msg +!/example-msg-files/test-embedded.msg # Reserved Folders /output diff --git a/CHANGELOG.md b/CHANGELOG.md index 1115fea5..c3a6977f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ -**0.58.1** +**v0.58.2** +* [[Advisory GHSA-6p33-92hw-4v86](https://github.com/TeamMsgExtractor/msg-extractor/security/advisories/GHSA-6p33-92hw-4v86)] [[Advisory GHSA-cxpg-prmv-g4pp](https://github.com/TeamMsgExtractor/msg-extractor/security/advisories/GHSA-cxpg-prmv-g4pp)] Fixed further attack vectors regarding the original advisory that were previously undiscovered. Most changes suggested in the new advisory were accepted as is, however a change was made to the handling of `srcset` to prevent it from being used at all if external files are not allowed. +* Changed `setup.py` to exclude `extract_msg_tests` from installed packages. +* Fixed an issue where the code for `MSGPath` was using a feature added in 3.9. + +**v0.58.1** * [[TeamMsgExtractor #493](https://github.com/TeamMsgExtractor/msg-extractor/issues/493)] Fixed unable to open embedded msg files due to `MSGBase.prefixList` still being used after it was removed. * Corrected the typing signature of `MSGBase.prefix`. diff --git a/README.rst b/README.rst index 98e0807e..80a2b904 100644 --- a/README.rst +++ b/README.rst @@ -268,8 +268,8 @@ your access to the newest major version of extract-msg. .. |License: GPL v3| image:: https://img.shields.io/badge/License-GPLv3-blue.svg :target: LICENSE.txt -.. |PyPI3| image:: https://img.shields.io/badge/pypi-0.58.1-blue.svg - :target: https://pypi.org/project/extract-msg/0.58.1/ +.. |PyPI3| image:: https://img.shields.io/badge/pypi-0.58.2-blue.svg + :target: https://pypi.org/project/extract-msg/0.58.2/ .. |PyPI2| image:: https://img.shields.io/badge/python-3.8+-brightgreen.svg :target: https://www.python.org/downloads/release/python-3810/ diff --git a/example-msg-files/test-embedded.msg b/example-msg-files/test-embedded.msg new file mode 100644 index 00000000..c0a3058c Binary files /dev/null and b/example-msg-files/test-embedded.msg differ diff --git a/extract_msg/__init__.py b/extract_msg/__init__.py index f4b505d7..245fd451 100644 --- a/extract_msg/__init__.py +++ b/extract_msg/__init__.py @@ -27,8 +27,8 @@ # along with this program. If not, see . __author__ = 'Destiny Peterson & Matthew Walker' -__date__ = '2026-10-31' -__version__ = '0.58.1' +__date__ = '2026-11-05' +__version__ = '0.58.2' __all__ = [ # Modules: diff --git a/extract_msg/constants/re.py b/extract_msg/constants/re.py index 47b62c02..da731ad3 100644 --- a/extract_msg/constants/re.py +++ b/extract_msg/constants/re.py @@ -4,7 +4,9 @@ __all__ = [ + 'CSS_IMPORT_EXTERNAL', 'CSS_URL', + 'EXTERNAL_URL_IGNORED_CHARS', 'HTML_BODY_START', 'HTML_SAN_SPACE', 'INVALID_FILENAME_CHARS', @@ -56,3 +58,14 @@ # Used for stripping urls from CSS. CSS_URL: Final[_RE_STR_TYPE] = re.compile(r'url\(\s*["\']?(https?:\/\/[^"\')\s]+)["\']?\s*\)', re.I) +# Used for stripping external @import rules from CSS. An @import that pulls in +# an external http(s) resource causes wkhtmltopdf to fetch it, so the whole +# at-rule is removed. This matches both the string form (@import "http://...";) +# and the url() form (@import url(http://...);), including any media-query list, +# up to the terminating semicolon. CSS_URL alone does not catch the string form. +CSS_IMPORT_EXTERNAL: Final[_RE_STR_TYPE] = re.compile(r'@import\b[^;]*?https?:\/\/[^;]*;?', re.I) +# Characters that HTML URL parsing ignores: leading C0 controls and spaces, and +# interior tab/CR/LF. These are removed before testing an attribute's scheme so +# a value like " http://..." or "ht\ttp://..." cannot smuggle a URL past the +# external-file check. +EXTERNAL_URL_IGNORED_CHARS: Final[_RE_STR_TYPE] = re.compile(r'^[\x00-\x20]+|[\t\r\n]') diff --git a/extract_msg/msg_classes/message_base.py b/extract_msg/msg_classes/message_base.py index 0a811d40..f5d40885 100644 --- a/extract_msg/msg_classes/message_base.py +++ b/extract_msg/msg_classes/message_base.py @@ -48,9 +48,9 @@ from ..recipient import Recipient from ..utils import ( addNumToDir, addNumToZipDir, createZipOpen, decodeRfc2047, findWk, - htmlSanitize, inputToBytes, inputToString, isEncapsulatedRtf, patchCss, - prepareFilename, rtfSanitizeHtml, rtfSanitizePlain, stripRtf, - validateHtml + htmlSanitize, inputToBytes, inputToString, isEncapsulatedRtf, + isExternalUrl, patchCss, prepareFilename, rtfSanitizeHtml, + rtfSanitizePlain, stripRtf, validateHtml ) @@ -612,15 +612,24 @@ def getSavePdfBody(self, wkPath = None, wkOptions = None, wkAllowExternalFiles: # Always decompose these, there is no good reason for these. if tag.name.lower() in ('iframe', 'script'): tag.decompose() - # Strip external files away only if they are not allowed. + + # Strip external files away only if they are not allowed. Each + # URL-bearing attribute is checked independently (not as an + # elif chain, which only ever stripped one per tag), the value + # is normalized first so a leading space or interior tab/newline + # cannot smuggle a URL past the check. elif not wkAllowExternalFiles: - if tag.get('href', '').lower().startswith('http'): - tag['href'] = '' - elif tag.get('src', '').lower().startswith('http'): - tag['src'] = '' - elif tag.get('data', '').lower().startswith('http'): - if tag.name.lower() == 'object': - tag['data'] = '' + # If anyone can show me a legitimate way for this to appear + # in an email with attachments and not just external URLs, + # then this can be restored. Extract-msg has no support for + # converting these to data source types anyways. + if tag.get('srcset'): + tag['srcset'] = '' + for attr in ('href', 'src', 'data', 'poster', 'background'): + if attr == 'data' and tag.name.lower() != 'object': + continue + if isExternalUrl(tag.get(attr, '')): + tag[attr] = '' # SECURITY: Last step, we need to go over all of the css data to ensure # no tracking images ONLY if images are disabled. diff --git a/extract_msg/msg_path.py b/extract_msg/msg_path.py index 689714b2..81518e76 100644 --- a/extract_msg/msg_path.py +++ b/extract_msg/msg_path.py @@ -82,4 +82,13 @@ def add(self, tail: str) -> MSGPath: """ Adds the tail to the last item in the path, without a separator. """ - return self.__class__(str(self) + tail) \ No newline at end of file + return self.__class__(str(self) + tail) + + if sys.version_info[1] == 8: + # Basically just use the future feature from pathlib. + def is_relative_to(self, *other) -> bool: + try: + self.relative_to(*other) + return True + except ValueError: + return False \ No newline at end of file diff --git a/extract_msg/utils.py b/extract_msg/utils.py index 90490494..2b712f48 100644 --- a/extract_msg/utils.py +++ b/extract_msg/utils.py @@ -28,6 +28,7 @@ 'inputToBytes', 'inputToString', 'isEncapsulatedRtf', + 'isExternalUrl', 'makeWeakRef', 'parseType', 'patchCss', @@ -830,10 +831,31 @@ def parseType(_type: int, stream: Union[int, bytes], encoding: str, extras: Sequ return value +def isExternalUrl(value: Optional[str]) -> bool: + """ + Whether a URL attribute value points at an external http(s) resource. + + The leading whitespace and control characters, and the interior tab/CR/LF, + that HTML URL parsing discards are removed before the scheme is tested, so a + value such as ``" http://..."`` (a leading space defeats a plain + ``startswith('http')``) is still detected as external. + """ + if not value: + return False + normalized = constants.re.EXTERNAL_URL_IGNORED_CHARS.sub('', value) + return normalized.lower().startswith(('http://', 'https://')) + + def patchCss(cssData: str) -> str: """ Takes the css data and patches out urls from it for security. + + Both external ``@import`` rules and external ``url()`` references are + removed, as either can make wkhtmltopdf fetch an attacker-chosen resource. + ``@import`` is handled first because its string form (``@import "http://..."``) + has no ``url()`` wrapper and would otherwise survive. """ + cssData = constants.re.CSS_IMPORT_EXTERNAL.sub('', cssData) return constants.re.CSS_URL.sub('none', cssData) diff --git a/extract_msg_tests/util_tests.py b/extract_msg_tests/util_tests.py index 02c9f98e..e6c8c3c5 100644 --- a/extract_msg_tests/util_tests.py +++ b/extract_msg_tests/util_tests.py @@ -21,6 +21,49 @@ def test_dictGetCasedKey(self): with self.assertRaises(KeyError): utils.dictGetCasedKey(caseDict, 'jjjjj') + def test_patchCss(self): + # External url() references are neutralized (pre-existing behavior). + self.assertNotIn('http', utils.patchCss('body{background:url("http://evil/x")}').lower()) + self.assertNotIn('http', utils.patchCss('@font-face{src:url(http://evil/x)}').lower()) + # External @import rules are neutralized in every form, including the + # bare-string form that has no url() wrapper (the fixed gap). + for css in ( + '@import "http://evil/x";', + "@import 'http://evil/x';", + '@import url("http://evil/x");', + '@import "http://evil/x" screen and (min-width: 1px);', + '@IMPORT "HTTP://EVIL/x" ;', + ): + self.assertNotIn('http', utils.patchCss(css).lower(), css) + # Local/relative references and ordinary text content are preserved; + # a text string that merely mentions a URL is not a fetch. + self.assertEqual(utils.patchCss('@import "theme.css";'), '@import "theme.css";') + self.assertEqual(utils.patchCss('p::after{content:"see http://x"}'), + 'p::after{content:"see http://x"}') + + def test_isExternalUrl(self): + externalUrls = ( + 'http://evil/x', + 'https://evil/x', + 'HTTP://EVIL/x', + ' http://evil/x', # leading space (the fixed gap) + '\t\n http://evil/x', # leading control characters + 'ht\ttp://evil/x', # interior tab, discarded by URL parsing + ) + nonExternal = ( + '', + 'cid:logo', + 'image.png', + '/relative/path', + 'data:image/png;base64,AAAA', + 'mailto:a@b.c', + ) + + for value in externalUrls: + self.assertTrue(utils.isExternalUrl(value), value) + for value in nonExternal: + self.assertFalse(utils.isExternalUrl(value), value) + def test_divide(self): inputString = '12345678901234567890' expectedOutputs = { diff --git a/extract_msg_tests/wk_tests.py b/extract_msg_tests/wk_tests.py index b04cb28b..ae4410da 100644 --- a/extract_msg_tests/wk_tests.py +++ b/extract_msg_tests/wk_tests.py @@ -75,6 +75,22 @@ def testWkSecurity(self): f'' ).encode(), ), + ( + "img-srcset", + ( + f'' + f'' + f'' + ).encode(), + ), + ( + "src-leading-space", + ( + f'' + f'' + f'' + ).encode(), + ), ( "link-css", ( @@ -98,6 +114,46 @@ def testWkSecurity(self): f'test' ).encode(), ), + ( + "css-import-string", + ( + f'' + f'' + f'test' + ).encode(), + ), + ( + "css-import-url", + ( + f'' + f'' + f'test' + ).encode(), + ), + ( + "css-font-face", + ( + f'' + f'' + f'test' + ).encode(), + ), + ( + "css-image-set", + ( + f'' + f'' + f'test' + ).encode(), + ), + ( + "css-no-quote", + ( + f'' + f'

x

' + f'' + ).encode(), + ), ] # We want to run all of the tests really quickly and then evaluate diff --git a/setup.py b/setup.py index d1d680e1..987fca42 100644 --- a/setup.py +++ b/setup.py @@ -39,7 +39,7 @@ author='Destiny Peterson & Matthew Walker', author_email='arceusthe@gmail.com, mattgwwalker@gmail.com', license='GPL', - packages=find_packages('.'), + packages=find_packages('.', exclude = 'extract_msg_tests'), py_modules=[main_module], entry_points={ 'console_scripts': ['extract_msg = extract_msg.__main__:main',]