-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Embedded SVG content type is text/plain #9644
Comments
@paulmedynski Thank you for your report! Version 1.6.5 is almost one year old, could you please try if this behaviour still occurs in the latest version 1.6.9? |
Or provide a sample message so we can easily try to reproduce. |
roundcube 1.6.5 is the latest available via default Debian 12 repos:
Here's a sample email that displays fine in Thunderbird and in previous roundcube versions: |
Thank you for noting that you're using the Debian package. Those are patched heavily, so the version number isn't really relevant. Could you post the URL of the bug report in the Debian tracker? (Or open one, if you didn't yet.) The Debian Changelog suggests that they picked the security fixes introduced in 89c8fe9, in which this line possibly introduced a bug: if (preg_match('~(javascript|jscript|ecmascript|xml|html|text/)~i', $ctype)) { @alecpl Shouldn't the ending slash be put after the closing bracket? Currently this evaluates to true also for |
Debian's patch is here: https://sources.debian.org/patches/roundcube/1.6.5%2Bdfsg-1%2Bdeb12u4/CVE-2024-42008.patch/, and contains the line unchanged. via |
Thanks for looking into this! Does your analysis above indicate that there is a bug in upstream roundcube that Debian included in their security patches? Or has Debian introduced the bug into their patches? Something else? I'm not sure what action I should take here. I will open a Debian bug if they introduced the issue, but if the issue comes from roundcube proper, then I think this issue suffices. Thoughts? |
Regression fixed. |
@alecpl Why didn't you change the Regexp but introduced a new special case? What is the Regexp supposed to match that would be broken by my suggested change? This doesn't look like clean code to me. @paulmedynski Debian only applied the patch, they didn't introduce it. As far as I know the standard procedure for Debian packages is to at first post a bug report into their tracker, so they can check if the problem is theirs or comes from upstream. This is due to them heavily changing the packages compared to upstream – e.g. if you hadn't mentioned that you installed Roundcube from the Debian package we might have searched for the culprit code in vain because the now changed code isn't even present in our v1.6.5. The Debian people might still be interested in a bug report to make them aware of a problem in their package, and of the fix in the upstream repository. |
The point of this code was to return all content, potentially containing evil code (scripts), as I excluded |
Prerequisites
Describe the issue
The fetch of an embedded SVG image within an email message results in the data being returned with content-type text/plain. For example, this URL:
GET /webmail/?_task=mail&_action=get&_mbox=Foo&_uid=618&_token=<redacted>&_part=1.2&_embed=1&_mimeclass=image
Results in a response with these headers:
As a result, the embedded images are not displayed.
What browser(s) are you seeing the problem on?
Chrome, Firefox
What version of PHP are you using?
8.3.11
What version of Roundcube are you using?
1.6.5
JavaScript errors
GET https:///webmail/?_task=mail&_action=get&_mbox=Foo&_uid=618&_token=&_part=1.9&_embed=1&_mimeclass=image 404 (Not Found)
PHP errors
Error log is empty.
The text was updated successfully, but these errors were encountered: