PHPfox-Official / phpfox-v4-issues

phpFox Public Bug Tracker
https://phpfox.com
30 stars 21 forks source link

Major security issue with ckeditor; including loading malicious code remotely and hijacking by redirecting. #2660

Open hamadax opened 5 years ago

hamadax commented 5 years ago

Important

Please follow this template!


What's happened?

Found an easy way to either hijack or load scripts remotely. Please see my recommendations under what is expected.

Steps to reproduce:

What's expected?

As I mentioned in another post, the lack of control over entering and managing html code in CKEditor is a major issue.

1- We need to be able to control what tags are allowed. This is done from the config file, but can be implemented in the admincp. I use another app that uses Froala and they do exactly that in the admin cp. They allow the admin to prohibit script tags and html elements. 2- We need to be able to allow the admin and staff to enter and edit entries in source view.

Browsers and Devices tested

(Example: Chrome on iPhone X, Safari on Macbook, Miscrosoft Edge on Windown 10, Firefox on Ubuntu 16.04, ...)

...

Server information

(Example: CentOS 7, php 7.1 apache)

...

phpFox version

(Example: phpFox 4.6.0)

...

Screenshots

...

prepperz commented 5 years ago

I've raised this issue before. See: https://github.com/PHPfox-Official/phpfox-v4-feature-requests/issues/768

hamadax commented 5 years ago

I've raised this issue before. See: PHPfox-Official/phpfox-v4-feature-requests#768

Thanks for the reply... I read your post and I am glad I am not alone here. Indeed we must be able to assign CKEditor packages to groups, but we also need a view source button for certain groups or all if HTML is allowed and the admin wants to enable it. The fact that anyone can slip malicious code or certain tags without notice is very serious and makes me think it's a complete deal breaker for a production site. The admin must be able to disallow a list of elements such as iframes and meta tags at minimum.

It is very much capable of doing both:

https://ckeditor.com/docs/ckeditor4/latest/guide/dev_acf.html https://ckeditor.com/docs/ckeditor4/latest/examples/sourcearea.html

I really hope this is resolved soon. I cannot turn off html and cannot take this risk on a live site.

harrison05 commented 5 years ago

Go to CKEditor in any app enter whatever script you want, even meta redirect or iframe click submit click edit click submit. Done. Page can now load scripts remotely via iframe, execute JavaScript, or even do a meta redirect.

We will check this issue, phpFox doesn't allow script tag in item content. About CKEditor control for each user groups (like feature request 768), we will review it in the future.

hamadax commented 5 years ago

We will check this issue, phpFox doesn't allow script tag in item content.

Hi Harrison, marking this issue as an enhancement is not really the way to go! Anyone can do the following: see videos:

1- Redirect and hijack a user to a different domain using simple meta tags:

https://youtu.be/yN9NBJ0AurM

2- Inject scripts remotely via iframe

Please see my recommendations to fix this. Admin needs to be able to prohibit a list of tags and groups such as admin and staff need to be able to examine the code via source view so they can remove unwanted code. Bear in mind that any user is able to to what I did above completely under the radar.

https://youtu.be/5XxXSXd4Brg

hamadax commented 5 years ago

Also bear in mind, I could have forced a malicious file onload with an iframe or redirected to phishing site. This is exactly how hackers managed to get into ebay's listings a few years ago. https://www.engadget.com/2016/02/04/ebay-bug-javascript-code/

There is absolutely nothing stopping any user from injecting code or redirects via iframes, nothing stopping meta redirects. Disabling HTML is not an option.

Update: This is not limited to blogs. I was actually able to load iframe in marketplace product description as well, the remote file had a JavaScript function to download a test virus file onload : https://www.eicar.org/?page_id=3950

Honestly, without an quick bug fix here, I am halting my project until I figure out a way around this. This is very risky.

harrison05 commented 5 years ago

We can update CKEditor to disallow meta/script and iframe tag to prevent XSS attacks. If we do this, when user edit content has those tags, CKEditor library will strip them out. But, the limitation is user can't use iframe button on the editor menu to add iframe URL to their content: Screen Shot 2019-05-16 at 10 05 45

prepperz commented 5 years ago

@harrison05 the point of https://github.com/PHPfox-Official/phpfox-v4-feature-requests/issues/768 is not to disable it entirely, but just to disallow certain usergroups such as regular users from having access to them while allowing authorized usergroups such as staff or phpfox site admin full access to use those feature such as iframe etc...

hamadax commented 5 years ago

@prepperz makes a good point. Disabling iframes globally will not allow embedding of legitimate media such as, but not limited to Tweets and YouTube videos. The best way to do this is to use the same logic in PHPFox... via groups and apps permissions combined, so each app/group combo has a set of CKEditor permission variables. For example, I want to allow bloggers to embed Tweets in the blogs app, but do not want anyone using iframes in a product description in the marketplace app. I think the same goes with scripting tags where they are used legitimately in embedding Tweets.

I know it is probably a lot of work, but I think the current limitations and vulnerabilities require looking at all possibilities and uses. For example, we are missing active content or the object tag from this conversation, which is another security issue. Currently, any user can embed Flash and Shockwave objects... This is why a real solution needs to give the admin the tools to limit these elements via the admincp, whether via a comma separated list or via configurable packages per permission group.

Another way which is a bit simpler is to use an open source whitelist app for CKEditor, which blocks all tags by default and the site admin can whitelist the tags they see fit in a comma separated set. This set of whitelist variables can be per group, per app, or both.

As far as meta tags, they can be disabled globally as I cannot think of a valid reason to use them.

Another issue which should not be neglected is the source view. The admin and staff need access to this.

Thought?

harrison05 commented 5 years ago

Ok. We will improve CKEditor, allow Admin control package for each user group like https://github.com/PHPfox-Official/phpfox-v4-feature-requests/issues/768

Regards

PhpFoxJohnJr commented 4 years ago

Don't know how I missed this...but this would be awesome!