U4-2524 - If tinyMCEImageHelper.DoResize fails, the src gets removed from the img tag

Created by James Vreeland 18 Jul 2013, 20:20:09 Updated by James Vreeland 22 Jul 2013, 12:07:06

umbraco.editorControls.tinyMCE3.TinyMCE Save() method calls tinyMCEImageHelper.cleanImages(parsedString), which calls tinyMCEImageHelper.(ht, out newWidth, out newHeight, out newSrc) when there is a "rel" attribute in the image tag and a new height and width are being set. The DoResize method uses and out parameter to set a newSrc string which is subsequently set as the "src" attribute. In the DoResize method this out parameter is initialized as "" (EmptyString) and only reset in a bool condition check for the existing file (line 134).
I believe this is a bug. The out parameter should be initialized to the original src value for cases that the bool check fails.

We are working with umbraco 6.0.2.

Comments

Sebastiaan Janssen 19 Jul 2013, 09:38:02

It shouldn't do, check that you have the src in the list of allowed attributes in umbracoSettings.config.

src,alt,border,class,style,align,id,name,onclick,usemap

Also make sure you upgrade your install to 6.0.7 due to this critical security problem: http://umbraco.com/follow-us/blog-archive/2013/5/1/security-update-two-major-vulnerabilities-found.aspx I can't stress this enough, you NEED to upgrade.


James Vreeland 19 Jul 2013, 12:10:03

Thank you, we do have the src attribute in the allowed settings. The issue is in the DoResize method which sets the out parameter newSrc = "". If the method fails to resize the image then newSrc is never set to an actual src. The calling method of DoResize (CleanImage) assumes the value in newSrc is good and sets the src attribute in the hashtable it renders the attributes from with this new value. So we ned up with an image tag having width and height but no src attribute.


Sebastiaan Janssen 19 Jul 2013, 13:54:43

I see. Do you have an example of how to make DoResize fail? It seems to me that that can be fixed then. Updated the title to reflect the actual issue.


James Vreeland 19 Jul 2013, 15:09:19

from line 134 in tinyMCEImageHelper if (fs.FileExists(orgPath)) { var uf = new UmbracoFile(orgPath); newSrc = uf.Resize(newWidth, newHeight); }

there are two potential fail points.
1.) FileExists should return true given the situation, but since it is being checked for then it may return false--in which case newSrc remains set to "". 2.) uf.Resize(newWidth,newHeight) can also fail, in which case I'm not sure what newSrc will be.

For our own instance it was failing at FileExists on our custom Azure File Provider.


Sebastiaan Janssen 22 Jul 2013, 11:57:36

I put in some error handling for both cases and will return the original source (plus do some logging) when these two errors occur. Revision: 319ba5c79109ed1b6221fd5ed45ad2cc82eff302


James Vreeland 22 Jul 2013, 12:07:06

Awesome, Thanks.


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 6.0.2

Due in version: 6.1.3

Sprint:

Story Points:

Cycle: