[Resuelto] Security concern: Multiple Lines field allows Script tag if HTML encoded

[Autor]

Mensajes

[Clifford]

Trying to save a script tag in Multiple Lines field type gets removed:

#tkcc-branding-container {
	font-size: 60px;
}
<script type="text/html" class="cliff-testing" id="scripttttttt"></script>

However, entering the value as HTML entities gets saved and even renders/appears as script tags (without entities, as entered):

#tkcc-branding-container {
	font-size: 60px;
}
<script type="text/html" class="cliff-testing" id="scripttttttt"></script>

Simply running html_entity_decode() turns that into an *actual script tag.

Here's a 4min video proof of concept: enlace oculto

In my custom code, I'm getting the value of the custom field (with HTML entities if in the 2nd example) but then escaping it myself by HTML entity *decode and then *strip tags, which leaves just the CSS code: enlace oculto (2min demo)

strip_tags( html_entity_decode( esc_html( $x ) ) );

Path forward:
Your own code removes Script tags unless they're encoded, which feels like a bug - either you allow them or you don't.
I understand sometimes people may *want to allow HTML entry so maybe when I create a Multiple Lines field you could add if ANY HTML tags are allowed (none should be the default) but allow to enable certain ones, such as "formatting tags" (Headings, Strong, Em, etc.) or "unfiltered, dangerous, possibly malicious Code" (such as Script) with a big warning of course.

[Juan]

Hi Clifford

Thanks for the feedback. This is Juan, Toolset development team leader. I would like to try to clarify this question.

As a default, we follow the WordPress standards. There is a capability named "unfiltered_html", which grants a reserved set of users the ability to post, and save, whatever content they want, without any restriction on HTML tags used, including script tags:
https://wordpress.org/support/article/roles-and-capabilities/#unfiltered_html
By default, this capability is only granted to admins (only to superadmins if the site is a multisite instalation). Those roles can indeed save script tags in post content, so they can also do it on our fields, and we do not run any safety check on them.

Our fields follow the same principle as WordPress for users without that unfiltered_html capability. In fact, if you add the same content directly to a post and save it, the script tag wil get removed and the encoded script tag will be kept. Because, as a principle, encoded tags are just characters and not actual tags that can or should be removed.

We include a setting in Toolset -> Settings -> Custom content -> Saving unfiltered HTML for users with higher roles, that allows you to be more restrictive: it bypasses the capability and removes the ability to save unfiltered HTML in our fields even if the current user can do that for native post content.

There is a list of HTML tags that users with restrictions can use. It is not totally straightforward to craft, but this core function should help:
https://developer.wordpress.org/reference/functions/wp_kses_allowed_html/
This of course means that some tags are indeed allowed in multiple line fields, by default and for all users. Any HTML that is valid for post content as per WordPress restrctions, is also valid for fields content, based on the current user role and capabilities. Bold, italic, etc... those elements are available as valid content to save.

Following your example, I see that data is safely stored and safely printed, unless you manually manipulate it and decode its entities. While I understand this might sound dangerous, whatever you do manually with the data is up to you: you could as well append a string printing a script to it. But the data in custom fields, multiple or single line fields, is safely stored and safely printed by default, following the same principles as data stored and printed in post content.

Now, should we also remove encoded unsupported tags? Well, WordPress itself does not do that on the post content, and I understand that there are legitimate usages for that encoded content. You might want to use a field to store a code snippet for example. Or some syntax in a language that is not HTML but is also tag-based. So in my opinion we should not restrict further what you can and can not include in your fields, given that the security level is the same as for the post content.

I hope this helps and mitigates the security concerns. Please let me know if I can help you with anything else.

Regards.

[Clifford]

I appreciate the thorough response. In short, if the action of saving removes non-encoded Script tags but not their encoded version, I'd consider it trouble, whether as a UI/UX inconsistency or as a security loophole. Additionally, the *appearance is of the entities rendered, unlike my input, and yet this is a plain text field type. Please consider this feedback.

[Juan]

Hi Clifford

Thanks again for the feedback.

I am sorry to say, but I need to disagree with the statement that saving encoded HTML is neither an UX/UI inconsistency nor a security loophole, for the same reasons I stated above:
- The content is stored securely in the database and is printed securely on frontend.
- The mechanism to decide which content is secure or not is following entirely the rules that WordPress forces upon native content, for the same user acess level based on the unfiltered_html capability.
- Encoded HTML is legitimate content and there is no security reason to remove it by design.
- Turning encoded HTML into actual HTML requires deliberate manipulation of the content, which grats the same access to the rendering logic as to already printing your own insecure content.

However, WordPress provides a rich API to hook and manipulate data on custom field saving time, so it would not be impossible to add a callback that manipulates the field value so all encoded HTML tags become actual HTML, and then removes all those tags. This is out of the scope of what Toolset does, but it is perfectly doable.

In any case, it is a very good practice to not trust data comig from the database. If you simply decode some content to be HTML but do not want HTML tags on it, stripping thos etags sounds like a good solution to me.

Hope this helps. Let me know if I can help you with anything else.

[Nigel]

Let me mark this as awaiting feedback.

[Clifford]

I appreciate your attention.