How to properly filter a form's data against XSS?

Question

Drupal 7.

I've got an (ECK module) entity form. The form's only field is a multi-value field of the field_collection (Field Collection module) type. The field collection contains two fields: a textarea, and a set of radio buttons. The user submits the form (AHAH), the entity is saved and the form is presented back to the user with the submitted values. For this project that is the intended (and necessary) behavior.

My (stripped down) validation handler is as follows:

/**
 * My custom validation handler.
 */
function my_module_eck_entity_form_validate($form, &$form_state) {
  $values = &$form_state['values'];
  $lang = $values['entity']->language;

  $fields = array('field_my_text', 'field_my_radios');
  foreach ($fields as $field_name) {
    // the values which get saved to the entity object
    foreach ($values['field_my_field_collection'][$lang] as &$item) {
      foreach ($item['entity']->{$field_name}[$lang] as &$child) {
        $child['value'] = filter_xss($child['value']);
      }
    }

    // the values which get output as part of the form on page-reload
    foreach ($form_state['complete form']['field_my_field_collection'][$lang] as &$item) {
      if (is_array($item) && isset($item[$field_name][$lang])) {
        foreach ($item[$field_name][$lang] as &$child) {
          if (is_array($child)) {
            $child['value']['#value'] = filter_xss($child['value']['#value']);
          }
        }
      }
    }
  }
}

It works and does what I want, but it is an ugly monstrosity that'll keep me up at night. There has got to be a nicer, cleaner way of doing this.

For the intents and purposes of this question, let's not discuss the pros & cons of Drupal's filter_xss() vs other tools - plenty of existing questions about that.

Answer 1

There's a huge issue with your implementation: it's destructive. Let's say a user writes about HTML, uses common comparison symbols or just uses the evil emoticon for despair: <o>. Changing data will make it harder for the user to later edit his post, it might even remove parts of it. Let's respect the user effort of typing into a textarea and hitting 'Save'.

So, what to do about XSS? Well, XSS is not dangerous in your database, it is only dangerous when displayed in a browser. So relax about saving it and worry about displaying it. Default formatters for text fields will use the text format settings to apply filters. Make sure the text formats available for your user are safe. And if you're writing your own formatter, don't forget to filter the output (preferrably by respecting the text format settings).

See https://drupal.org/node/28984 and https://drupal.org/documentation/modules/filter for more on this topic.

Answer 2

Ended up with a different solution than what I originally posted. The following code adds a "Perform XSS validation" checkbox on the edit-field pages for all text fields, and then performs validation on those fields. If there are validation issues, an error is thrown and the form doesn't submit.

  /**
   * Extends hook_form_alter().
   */
  function hook_form_alter(&$form, &$form_state, $form_id) {
    if ($form_id == 'field_ui_field_edit_form' && in_array($form['#field']['type'], array('text', 'text_with_summary', 'text_long'))) {
      $form['instance']['xss'] = array(
        '#type' => 'checkbox',
        '#default_value' => isset($form['#instance']['xss']) ? $form['#instance']['xss'] : 0,
        '#title' => t('Perform XSS validation'),
      );
    }
  }

  /**
   * Extends hook_field_attach_validate().
   */
  function hook_field_attach_validate($entity_type, $entity, &$errors) {
    $info = entity_get_info($entity_type);

    if (!isset($info['entity keys']['bundle']) || !$info['entity keys']['bundle']) {
      return;
    }

    $bundle_key = $info['entity keys']['bundle'];

    $lang = isset($entity->language) ? $entity->language : LANGUAGE_NONE;
    $fields_list = field_info_instances($entity_type, $entity->{$bundle_key});

    foreach ($fields_list as $field_name => $field_settings) {
      if (isset($field_settings['xss']) && $field_settings['xss']) {
        $field_items = field_get_items($entity_type, $entity, $field_name, $lang);

        foreach ($field_items as $delta => $item) {
          if (isset($item['value']) && $item['value'] != filter_xss($item['value'])) {
            $errors[$field_name][$lang][$delta][] = array(
              'error' => $field_name,
              'message' => t('Field contains invalid content.'),
            );
          }
        }
      }
    }
  }

Answer 3

I'm in favour of Yuriy's solution, since it prevents insertion of vulnerable code in the backend + proper user feedback in case something would have been pasted "accidently".

The embedded code sample is not completely working though for all entity types. For example "user" entity does not have a bundle entity key defined which makes it impossible to XSS check custom defined user fields.

Also, the applied method to expose proper errors to the user is not working when validation is done on node/%/edit form submissions, therefore replaced with more common form_set_error() function.

Here's a slightly adapted version of the snippet, working for both node entities as others :

function hook_field_attach_validate($entity_type, $entity, &$errors) {

  $info = entity_get_info($entity_type);
  if (isset($info['entity keys']['bundle']) && $info['entity keys']['bundle']) {
    $bundle_key = $info['entity keys']['bundle'];
    $bundle = $entity->{$bundle_key};
  }
  else {
    $bundle = $entity_type;
  }

  $lang = isset($entity->language) ? $entity->language : LANGUAGE_NONE;
  $fields_list = field_info_instances($entity_type, $bundle);
  foreach ($fields_list as $field_name => $field_settings) {
    if (isset($field_settings['xss']) && $field_settings['xss']) {
      $field_items = field_get_items($entity_type, $entity, $field_name, $lang);
      foreach ($field_items as $delta => $item) {
        if (isset($item['value']) && $item['value'] != filter_xss($item['value'])) {
          form_set_error($field_name, t('Field "@label" contains unallowed text elements.', ['@label' => t($field_settings['label'])]));
        }
      }
    }
  }
}