angular-froala icon indicating copy to clipboard operation
angular-froala copied to clipboard

XSS Vulnerability

Open kp-thibaut opened this issue 6 years ago • 10 comments

The ngModel.$isEmpty function bypass the native froala security cleaning method, by executing the content of value with the JQuery function.

In my case, I just reuse the froala native html.clean method to fix it.

Like this:

ngModel.$isEmpty = function (value) {
	if (!value) {
		return true;
	}

	value = element.froalaEditor('clean.html', value, [], [], false);

	var isEmpty = element.froalaEditor('node.isEmpty', jQuery('<div>' + value + '</div>').get(0));
	return isEmpty;
};

Example of XSS injection concerned: Script URI scheme XSS test<img src="javascript:alert('XSS')">

kp-thibaut avatar Mar 07 '18 11:03 kp-thibaut

@kp-thibaut do you think you could make a PR?

stefanneculai avatar Mar 07 '18 11:03 stefanneculai

yup, but I will make more tests before.

kp-thibaut avatar Mar 07 '18 12:03 kp-thibaut

@stefanneculai I'm ready to push my branch, but I haven't the right to do this. BTW, I have fixed some lint issues to and all your tests are down due to new JQuery version (3.3.1) by the froala dependencies.

AS the change is not invasive, I push it without testing it via grunt. I've made some tests by my side.

kp-thibaut avatar Mar 07 '18 15:03 kp-thibaut

@kp-thibaut please make a PR: https://help.github.com/articles/creating-a-pull-request-from-a-fork/.

stefanneculai avatar Mar 07 '18 15:03 stefanneculai

@stefanneculai it's done.

kp-thibaut avatar Mar 07 '18 17:03 kp-thibaut

Thanks. We're going to review it shortly and merge it.

stefanneculai avatar Mar 08 '18 17:03 stefanneculai

@stefanneculai any news ?

kp-thibaut avatar Mar 29 '18 13:03 kp-thibaut

@stefanneculai, I follow your advancement but I haven't seen any change for the XSS Vulnerabilities.

I'm pretty concerned by this issue, as froala is deploy on all my customers. Security is one of my main priority. And you still have a huge vulnerability for ALL XSS attacks.

The fix I propose and present on my PR is in production for 4 month on every of my customer.

Can you reply to me with any info or update ?

kp-thibaut avatar Jun 29 '18 13:06 kp-thibaut

Has this been fixed yet?

RHinderiks avatar May 06 '19 07:05 RHinderiks

Not even close. They have fixed a XSS issue for the WYSIWYG editor, but here the problem come from the Directive on this line: return element.froalaEditor('node.isEmpty', jQuery('<div>' + value + '</div>').get(0));

Even if not attached to the body, there is a dom element create with the content of value, but this one is never filtered or secure, so there is still a possibility for XSS attacks.

When calling this jQuery('<div>' + value + '</div>') the malicious JS is called. They need to secure the value before using it. You can avoid those XSS attack by using the html.clean method of froala: value = element.froalaEditor('clean.html', value, [], [], false); and then value is secure.

kp-thibaut avatar May 09 '19 09:05 kp-thibaut