Support

Account

Register Login

Home › Forums › Bug Reports › Field saving does not respect unfiltered_html capability

Solving

Field saving does not respect unfiltered_html capability

  • Users assigned roles without the unfiltered_html capability are able to save html markup in ACF fields. This opens up a site to XSS vulnerabilities by allowing any user to paste in potentially malicious code, rather than requiring users with the expressed capability only.

    Steps to Reproduce:
    1. Create an ACF text/textarea/wysiwyg field on a page.
    2. Create a new user with role Author. Authors by default do not have the unfiltered_html capability.
    3. Create a new page as the new user, with <script>alert('This should not happen!');</script> as the ACF field content.
    4. View the page, the script runs.

    Expected Behavior: For users without the unfiltered_html capability, script tags should be stripped out and the field content should display as alert('This should not happen!').

    Proposed resolution: ACF fields should check if the user has the unfiltered_html capability on save, and if not, run field content through wp_filter_post_kses().

  • Hi @benabaird

    Thanks for the bug report and sorry for my delayed reply.
    This is a good idea and will be easy to implement.

    Please edit the file “includes/form.php” and find the line ~160:

    
// action
do_action('acf/save_post', $post_id);

    Then change it to this:

    
// Filter $_POST data for users without the 'unfiltered_html' capability.
if( !current_user_can('unfiltered_html') ) {
	$_POST['acf'] = wp_kses_post_deep( $_POST['acf'] );
}

// action
do_action('acf/save_post', $post_id);

    We have tested this and can confirm it works correctly for author users who don’t have the ‘unfiltered_html’ capability:
    https://codex.wordpress.org/Roles_and_Capabilities#unfiltered_html

    Let me know your thoughts on the fix and I’ll aim to release a patch shortly.

    Thanks
    Elliot

  • @elliot Sound update for most all use cases – but, would you consider adding a filter on this so we can choose fields to bypass kses? or is there some other way currently?

    Use Case: We use a text area field on the page editor for “custom scripts”, which only outputs to logged out users. For now, we have rolled back this release until we come up with a work around.

    Thanks!

  • Hi @chriscarr

    Thanks for the comment. I’m happy to add this in.
    For naming, maybe something like ‘acf/allow_unfiltered_html’?
    You could hook into this and override the boolean value.

  • Yeah, that would definitely allow us to bypass the check and keep up to date 👍

    Honestly I had not really looked at the implementation when I suggested this, and was thinking this was being run later – per value rather than on the entire $_POST['acf'].

    I suppose for our use case we would then want to add our own conditional KSES implementation later in the save_post process, like in acf/pre_update_value.

    (Just droppin these notes in case someone else is in the same situation someday)

    Thanks!

  • Hi @chriscarr

    Just wanted to let you know I’ve added in a new filter for v5.7.10 called “acf/allow_unfiltered_html”.

    You can hook into this and return true/false to allow/prevent unfiltered HTML.

  • Hmmm – maybe I’m thinking about this all wrong, but how would one filter acf/allow_unfiltered_html on only 1 field on a page? It seems like it’s either an all or none type situation.

    My use case is the same as others – I created a textarea field for code, but it gets filtered when a non-super-admin updates the page. I want admins to be able to edit that field, and all other users can edit other parts of the page (which will be kses’d), but the code field would not have it removed. I have the code field hidden from non-admins (good enough to prevent problems for now, although I might can improve on it with a validation hook that checks for updates by non-admins).

  • It is an all or nothing thing, run on the entire $_POST[‘acf’] array before acf/save_post is run.

    To allow it on just one field you would need to allow it on all fields and then you’d need to create your own filters for every other field except the one you want to allow. You’d have to hook in and run your own filter on $_POST[‘acf’] before ACF does… I’m not exactly sure how you’d do that. Possibly on the “init” hook with a low priority.

  • How about true on ‘allow_unfiltered_html’ filter, and then run wp_kses() (or wp_kses_deep()?) on !code_field_id on the ‘acf/update_value’ filter?

  • 1- Is there a security issue between acf_save_post(), and acf_update_value() if you let wp_kses go that far before running? I don’t see anything, personally, but all functions include acf_save_post, _acf_do_save_post, acf_update_values, acf_get_field, acf_update_value.
    2- Is this going to be a potentially very heavy hook to run?

  • To your first question, yes, you could do that.

    to the second post
    1) You mentioned “super admin”. It is impossible now, due to a change in WP to give anyone other than super admins “unfiltered_html”. Or at least nearly impossible. You can give them the permission, but it will be ineffective on the main content area for the site. I’m not sure if I figured out a work around for this the last time I ran into it or not. ACF seems to be working around this for acf fields.
    2) It would be no heavier to run wp_kses() on each field than it would to run it on all fields at once, I don’t think.

  • OK. I’m fine with normal content areas being filtered. Although, the nature of the filter seems kinda odd to me though, because if the super admin adds code to a post, then a non-super-admin comes along and updates the same post, the stuff the super-admin added will disappear.

    Anyway, this is the filter I made. It depends on the code field ending with ‘code’ in its name.

    // remove the html filter on all ACF fields
add_filter('acf/allow_unfiltered_html', 'wilirius_acf_allow_unfiltered_html_all_fields');
function wilirius_acf_allow_unfiltered_html_all_fields() {
    return true;
}
// re-apply filter to non-code fields
add_filter('acf/update_value', 'wilirius_acf_disallow_unfiltered_html_non_code_field', 1, 3);
function wilirius_acf_disallow_unfiltered_html_non_code_field($value, $post_id, $field){
    if(substr($field['name'], -4) !== 'code') {
        if(
            $field['type'] !== 'tab' &&
            $field['type'] !== 'group' &&
            $field['type'] !== 'repeater' &&
            $field['type'] !== 'flexible_content' &&
            $field['type'] !== 'clone'
        ){
            $value = wp_kses_post($value);
        }
    }

    return $value;
}

  • Are there any other field types that should be checked against that it would be bad to run wp_kses_post() on?

  • NM my remark about super admins and unfiltered_html… the problem I ran into was with “unfiltered_uploads”

  • *remove

  • In response to other field types, any field that stores an array could give you issues, but it’s been a while since I looked at that.

  • Yes, unfiltered html is a pita and a little stupid because you’re right, if a super admin adds something and an editor edits the post it will be removed. It’s best to find other ways to allow them to add code when needed. The last time I had an issue is was with using GA and the client wanted to be about to add GA events to links. Now we use tag manager and that problem has been eliminated.

    After discussing this I realize that this is a new way the ACF is handling fields. I’m going to have to make some adjustments to several sites.

  • OK – I added if( !is_array($value) && !is_object($value) ) { before the wp_kses, and else writes to the error log so I’m aware that it is something that might need to get covered.

  • I used this snippet for my multisite too. It looks like it stopped working on WP 5.3.2. You still use these filters?

  • I also need help with this… Non admins in my multisite have unfiltered_html capability but they are still not able to save iframe’s in textarea inside repeater.

  • Does filtering acf/allow_unfiltered_html alone not work?

    Since you’re on a multisite, do superadmins have trouble as well?

  • Multisite unfiltered_html https://kellenmace.com/add-unfiltered_html-capability-to-admins-or-editors-in-wordpress-multisite/

  • I can confirm (ACF PRO v5.8.7) that this is still an issue for non-admins on a multisite. Filtering the ‘acf/allow_unfiltered_html’ does work and the above link does also fix the problem.

    But both fixes are workarounds and the capability still is being ignored. Besides that the above fixes expose vulnerability for all fields where I would like to only allow this for a specific field.

    Looking forward to a field-specific (in combination with role) fix.

  • Try this https://github.com/crowdfavorite-mirrors/wp-unfiltered-mu/blob/master/unfiltered-mu.php

Viewing 24 posts - 1 through 24 (of 24 total)

The topic ‘Field saving does not respect unfiltered_html capability’ is closed to new replies.