CKEditor removes the "style" attribute

I have exaxctly the same problem. Tried all things described in first comment, the only difference - I have CKEditor v3.6.5.7647

Check this ticket, it's Drupal 8, but the same problem goes to Drupal 7 also: https://www.drupal.org/node/205160
In my case problem was caused by "Limit allowed HTML tags" (filter_html) filter, which is a part of core Filter module. As a solution I have created new filter based on code of filter_html filter. The actual problem is in _filter_xss_attributes() function, there is line in code that contains:

$attrname = strtolower($match[1]);
$skip = ($attrname == 'style' || substr($attrname, 0, 2) == 'on');

If $skip is TRUE, then your attribute will be removed. As you see style attribute is hardcoded, so no chances to make it work without coding or using alternative filter.

Wysiwyg currently sets ACF off by default to prevent it from removing existing content on upgrade.

I would recommend using HTML Purifier or WYSIWYG Filter module instead of "Limit allowed HTML tags".

Running without any kind of filter is dangerous, and the Core filters will always remove style attributes for security reasons.

(Btw, modifying config.js will have no effect since Wysiwyg doesn't load it. Look into implementing hook_wysiwyg_editor_settings_alter() in a small custom module if you need more precise control of the editor settings.)

@tracker2k Thanx for your investigations, it was strting point for me.

Initially only working solutions for me was CKEditor Filter.

But I wanted to find root of evil.
So after long drilling into core`s "Limit allowed HTML tags" filter, I found that filter_xss_bad_protocol() is used on all attributes which are parsed out of HTML... yes, on 'style' attribute also.
And of course "font-size:30px;" looks like extremely bad protocol... so just wiped out.

As solution I have copy/pasted "Limit allowed HTML tags" into custom module, renamed it and modified (custoly renamed) filter_xss_attributes() so:

1. Added

           $skip = ($attrname == 'style' || substr($attrname, 0, 2) == 'on');
   >        $skip_style = ($attrname == 'style');
   

   so that I could have additional info about working with 'style' attribute.

2. Made additional checking with storing 'style' as is into attributes-preserving array.

         case 2:
           // Attribute value, a URL after href= for instance.
           if (preg_match('/^"([^"]*)"(\s+|$)/', $attr, $match)) {
   >          if ($skip_style) {
   >            $attrarr[] = "$attrname=\"$match[1]\"";
   >          } 
   

3. Turned off core`s "Limit allowed HTML tags" and enabled my custom filter.

That is all :)

@q11q11, There is no 'evil' here other than the one filter_xss_attributes() tries to protect you from by removing style attributes. It's designed to be a very simple filter which removes all styles to prevent any possibility of XSS injections via malicious URLs in style properties.
If you want an alternative with more configuration options, which allows you to leave the style attribute in and whitelist the font-size property, see the solution I mentioned in comment #5.

No need for a custom module with hardcoded values as you can configure WYSIWYG Filter completely through the GUI per format, and even export the editor profile and format configuration with Features.

@TwoD
Problem is that filter_xss_bad_protocol() is appied onto style values, and what does this function do?
Inside filter_xss_bad_protocol() this one drupal_strip_dangerous_protocols() is applied onto style values.
If we look deeper we see that drupal_strip_dangerous_protocols() has this array('ftp', 'http', 'https', 'irc', 'mailto', 'news', 'nntp', 'rtsp', 'sftp', 'ssh', 'tel', 'telnet', 'webcal'); as allowed protocols.
So if there will be some malicious URLs, they definitely will be using one of ALLOWED protocol to intrude it.
I don`t see any point in wiping all other styles, but passing styles with malicious code thru allowed URLs.

That function is only designed to remove things like the javascript: "protocol" from URLs. The filtering process is more complex than that alone. Style attributes are completely skipped earlier in the process, by design, as noted in comment #3.

As the filter description says: "It will also remove harmful content such as JavaScript events, JavaScript URLs and CSS styles from those tags that are not removed."

It is designed to be simple and effective, without exposing new users to a ton of options which could actually turn it into a "non-filtering" filter if they aren't careful. If you want that, use the other filters available in contrib, such as WYSIWYG Filter.

Exactly, removing potentially malicious protocols from urls, and not for removing "font-size" from "font-size:30px;" styles which just breaks it.
Looks like using microscope for hammeing screws... to my point of view.

About module`s description - probably it`s is outdated or just wrong, else this thread won`t be ever created.

I have debugged filtering mechanism quite a deep.
It works like parsing whole html and tearing off parts of it and then parsing those parts deeper and tearging it into more parts.
Then deciding which part is ok to be remained.
After all that mechanism tries to combine it all back together with implode().

And concerning inline styles - $skip variable from #3 doesn`t prevent style attribute to be really skipped.
It only prevents slack-baked mechanism from breaking stylings.

Let me show an example and let`s assume we turned off style checkings from $skip in #3.
Imagine we have 2 attributes in one < div > tag: class="class-1" and style="color:red".
Filter will tear off 'class="class-1"' and 'style="color:red"' out of div.
Then it will split into 'class' and "class-1", then 'style' and "color:red".
Then it applies filter_xss_bad_protocol() onto "class-1" and "color:red".
With "class-1" it returns "class-1" seems ok, pass it.
With "color:red" it returns "red", malicious protocol 'color' removed, seems ok, move along.
And after combinind it all together we have funniest part < div class="class-1" style="red" >.
How much meaning does browser see in the style="red"? answer - 0!
That is why $skip for style is enabled, nothing else.
More to say - < img src="http://example.com/malicious.gif" > is more dangerous from XSS point of view, but it won`t be wiped out, coze "http" is allowed protocol.

So to my point of view - filter.module maintainers just didn`t want to deal special-case-style-filtering, so they just stated in code - "styles are wiped by design", but mechanism is really crippled and using not proper ways of dealing inline style attribute.

It's not just about filtering out URLs, it's actually meant to completely obliterate style tags. Yes, it passes the style value through filter_xss_bad_protocol(), and that call should really have been optimized to not do that when $skip is set.

If you have access to style an element any way you want you could easily click-jack someone else reading something you posted just by inserting something like this:

<a href="http://example.com/attack.html" style="display: block; z-index: 100000; opacity: 0.5; position: fixed; top: 0px; left: 0; width: 1000000px; height: 100000px; background-color: red;"> </a>

Since there is no WYSIWYG editor in D7, and the whole format/filter system is designed to be able to generate safe HTML markup from tokens or stripped down syntaxes like Markdown or BBCode, they chose to let the filter be as strict as possible.
This is also why img's aren't allowed by default. Browsers were really crappy about discovering/blocking attacks in inline styles when D7 came out and you can still use the click-jacking attack above today.

I`m using CKEditor, so I have correct markup.
Also, descriptions of XSS attacks done thru style attribute (that I have found) are referencing IE6 and Netscape... How relevant is IE6 nowdays?
I couldn`t get any working snippet at all.
Anything of that kind are just not working in modern browsers.
So why to keep obsolete protections done with wery weird and not direct solutions?

You may be using CKEditor, but that is irrelevant. Anything performed on the client through JavaScript is easily bypassed.

An attacker can simply disable any editor put on a textarea, post the example snippet I gave into a comment on your website and wait for someone to view the unfiltered markup. The next click they make will be caught by the link, including in modern browsers like Chrome or Firefox, and they'll be taken to whichever URL the attacker desires. If the visitor is logged in on the target website and it's got some flaw, they could trigger the visitor into performing some dangerous action using their account.
Drupal requires confirmation steps on most actions for this very reason, so click-jacked accounts won't accidentally delete a node or change their password just via as single GET request.

Had I not posted the snippet in a <code> block and Drupal.org was running without a filter, you would have been subject to this when viewing my comment and attempting to reply.

I see, you are talking about anonymous commenting and so on.
This don`t have css and yet is extradangerous < a href = "http://example.com/attack.html" > click here for free money < / a > ...
So of course anything that is not plain text is dangerous.

I`m talking about editors, content creators, couple of trusted users who knows what to do with CKEditor.

BUT...
We have already moved far from main idea that I was saying before:
- wiping CSS with function designed for completely another purpose.
To my point of view it`s bad manner if done accidently and lazyness if done intentionally.
And it`s in core.

Yes, it's mainly about anonymous posting, but if you want to be strict you can never trust user input.
A trusted non-malicious user can be infected, or leave a logged in account unattended by mistake. If the worst thing an attacker can do is enter profanity or just delete content it's a lot less of a concern than if they in turn can infect all your visitors, or maybe even steal admin credentials when they go to clean up the mess.

If your editors need a higher level of freedom, they also require a higher level of trust, so you can possibly disable the filter for them. You could also move them to a more configurable filter if you don't really trust them with all the keys to the kingdom. Drupal does not aim to cover every use-case explicitly, but it does cover both ends of this spectrum with enabling safe and reliable filtering for anonymous posting and unrestricted access for trusted users. Contrib filled in the blanks as the needs grew.

Yes, filter_xss_bad_protocol() wipes CSS properties when calling drupal_strip_dangerous_protocols(). It is however not really intended to run for CSS property name/value pairs, as the only place which calls it ignores its output in that case. Everything else calls drupal_strip_dangerous_protocols() directly and only on URIs.

The developers weren't lazy and knew what they were doing. They were thorough enough to go research and find existing filtering code which fulfilled the needs of the filter they were writing and incorporated that into Core. That code fulfills its purpose of destroying inline styles to protect you when the filter is used. If you don't want that level of safety you don't apply complete XSS filtering.

The threat is real and the core contributors did likely not have the need to prioritize a 100% configurable system because Core already now covers the two basic modes (XSS filtering on or off) with, in comparison to something like htmLawed, simple and easy to understand filtering code.

While investigating this I also noticed the process for that filter is kept the same in D8. In addition, D8's editor does not allow inline styles at all when used in combination with an XSS filter.
A module could override this behavior, as I noted in the linked answer, but the devs have made sure you do really need to know what you are doing, and why, when overriding it. Core is safe by default, and you can still chose to do something else.

I believe you are somewhat missing one point of the filtering system. Other than giving you optional protection from possibly malicious or malformed input with the "Limit HTML tags" filter, it also makes it possible to use safe ways to let users generate markup.
If you need your users to be able to change the font-color and you want to be guaranteed they can't use any of the other style attributes, you could write a relatively simple custom filter which turns This is my [color=#ff0000]red[/color] content into This is my <span style="color:red">my</span> content, which is exactly why the BBCode module exists.
If you want to give your editors a toolbar as well, add a WYSIWYG editor with a BBCode output plugin. The editors don't need to know either HTML or BBCode to get their styling done without the risk of introducing, or being subject to, XSS attacks.

Ideally, you should need almost no inline-styling. Applying classes and "block format" types (paragraph/header/span/div/) and filter placeholders will let your theme decide how it actually gets rendered, and you can change it at any time without modifying legacy content in the database.

Drupal shouldn't make this decision for us. If we want to allow privileged users access to a filter that allows the style attribute, nanny Drupal should not override our decision. This is poor design.

I ran into the same issue. In my case, it was due to core file /core/lib/Drupal/Component/Utility/Xss.php.

@16davidfells81@gmail.com, as I mentioned in #15, you can override it if you know what you are doing, but it must be safe by default as that matches almost every use case.

@rajeevgole. Applying that patch would be a security risk, please read #15 to see why. (It's also for D8 core so not really relevant here.)
TL:DR; Inline styles allow malicious URLs (and other weird stuff) which end up executing JavaScript and thus allow for XSS attacks. If you have HTML Filtering enabled, all styles are removed by design as they can not be trusted, and allowing just a subset of inline styles while staying secure is difficult. Need more control? Use another filter - or none at all - as this patch essentially breaks the XSS protection that filter implements. (Wysiwyg Filter module for D7 does this, but it requires precise configuration.)

New patch working with drupal 8.9.14

Added a patch for Drupal version 9.3.0

Just adding another reminder here that applying the latest patch would also be introducing a security risk for your site. Use with caution.