I know there are a number of issues about this and the maintainers of this module have firmly expressed that they have no intention to support the whitelisting of the elements defined in wysiwyg_filter_get_elements_blacklist() (primarily for security reasons). I understand and respect that.

Sometimes, however, this module is being used in a context in which users of a given input filter are fundamentally trusted (perhaps they're all employees of a small company) but you want to limit available markup so as to avoid crazy font colors, inline CSS, or other poor markup choices. But these are trusted users, so you're not too concerned about closing all possible attack vectors via tags like object, etc. So I propose employing an alter hook to allow the modification of the blacklist. This prevents less-knowledgeable users from inadvertently allowing these elements, but gives more-knowledgeable devs a way to leverage this very useful module for a wider variety of use-cases than is currently possible.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

azinck’s picture

Simple patch attached. If the maintainers are ok with this approach then we can add big scary comments about using the hook at your own risk, and maybe refactor a bit so that the name of the text format is passed to the new alter hook.

azinck’s picture

Status: Active » Needs review
drasgardian’s picture

I like this approach to allow hooks to alter the blacklist list.

azinck’s picture

I noticed I uploaded a crufty patch with stuff from my environment. Clean patch below.

vinmassaro’s picture

This is a must for us in order to get the Drupal embed media button working in CKEditor. It allows you to paste in embed code from media providers, which is usually an iframe. Can you explain how I implement the alter in my module after applying this patch? Thanks.

vinmassaro’s picture

Status: Needs review » Reviewed & tested by the community

Nevermind, figured it out:

function my_module_wysiwyg_filter_elements_blacklist_alter(&$blacklist){ 
  $blacklist = array(
    'applet',
  );
  
  return $blacklist;
}

This works great. Thanks!

amaasbier’s picture

I could not get the patch to work - even if I clear my Drupal cache.
Is there maybe something else I have to configure? Could not find any new options in the modules section nor the format options.
What does this line do?
drupal_alter('wysiwyg_filter_elements_blacklist', $blacklist);

vinmassaro's solution seems insecure to me.
Thanks for the patch. I'd be happy to apply it.

azinck’s picture

amaasbier: this patch provides an alter hook to let you change the blacklist provided by Wysiwyg Filter. It does not add anything to the configuration or user interface. You must implement hook_wysiwyg_filter_elements_blacklist_alter() in your own module and modify the array of elements that are to be blacklisted. vinmassaro illustrates one way to use this hook (though he need not return the variable at the end).

However, I would argue that if you don't know how to use this patch that perhaps you shouldn't be doing it at all. Changing the blacklist may expose attack vectors that the module's maintainers feel they haven't adequately protected against. Please use at your own risk.

amaasbier’s picture

Thanks for the quick reply.
I think you are right about me not being experienced enough to fiddle with a module's source code. Since I only wanted to embed videos from well known providers, I found a more fitting module for my needs: "Media: Vimeo" and "Media: Youtube". I did not manage to display videos inline yet, but it's at least a step forward.
Thanks for your help anyway. It's much appreciated.

TimG1’s picture

+1 #4 worked great, thanks for the patch.

-Tim

PierreV’s picture

#9 : I used #4 patch to allow iframes, with this (in wysiwyg_filter.pages.inc, function wysiwyg_filter_filter_wysiwyg_process) :

	//Match iframes
	$allowed = array(
		'http://www.youtube.com/embed/',
		'http://www.dailymotion.com/embed/video/'
	);
	preg_match_all('%<iframe[^>]*src=["\']([^"\']*)["\'][^>]*><\/iframe>%isU', $text, $matches);
	if (!empty($matches[1])) {
		foreach($matches[1] as $key=>$url) {
			$flag = false;
			foreach($allowed as $value) {
				if (strpos($url, $value) === 0) {
					$flag = true;
				}
			}
			if (!$flag) {
				$text = str_replace($matches[0][$key], '', $text);
			}
		}
	}

It's just a quick a dirty way to allow only some medias, but it works.

azinck’s picture

I do not recommend following ZeSenki's approach in #11 as he's hacking the module. I know that the module maintainers are very sensitive to security (a good thing) so I think it'd be best not to pollute this already questionable issue with far more rogue suggestions.

maximpodorov’s picture

Any plans to commit the patch? Is really works.

seanB’s picture

Patch in #4 works perfect! Hope it gets added to the module.

waitnsee’s picture

I've applied the patch successful but have no idea how to use the hook_alter or change the list of elements in the blacklist. Anyone can give me some advice??

maximpodorov’s picture

Thir real module adds IFRAME as the acceptable tag for wysiwyg filter configuration:

function i_wysiwyg_filter_wysiwyg_filter_elements_blacklist_alter(&$blacklist){        
  $new_blacklist = array();
  foreach ($blacklist as $value) {
    if ($value !== 'iframe') {
      $new_blacklist[] = $value;
    }
  }
  $blacklist = $new_blacklist;
}
heylookalive’s picture

+1 for patch on #4.

I agree that secure out of the box but we should be able to allow whatever elements suit the site, e.g. iframes.

askibinski’s picture

#4 works like a charm in combination with a custom module.

lordzik’s picture

edit:
I've removed "object" and "param" from blacklist and allowed object[*] and param[*] but still embeded video does not show properly. It looks like value of flashvars param is partialy removed even if param[*] is allowed.

Why?

edit 2:
Oh, if somebody else need to put flash video on site that uses wysiwyg filter - just reorder filters on "admin/settings/filters/1/order" so that "SWF Tools filter" appears after "WYSIWYG Filter" :)

muschpusch’s picture

+ 1 for committing!

seanB’s picture

Looks like we all agree that this is a great fix to allow more flexibility. Is there a reason to not commit this?

marcoscano’s picture

+1 for commiting

the patch in #4 works as expected

rooby’s picture

Issue summary: View changes

+1 for this also.

Re #16, you can do things like that without having to loop using array_search() or array_keys().

rooby’s picture

#6 is not a good solution for allowing media as it also allows a bunch of other things like forms that you don't need for media.

It also shouldn't return $blacklist at the end, because it is passed in by reference.

This is sufficient for me to remove only the iframe element:

<?php
/**
 * Implements hook_wysiwyg_filter_elements_blocklist_alter().
 */
function MODULE_NAME_wysiwyg_filter_elements_blacklist_alter(&$blacklist) {
  // Remove iframe from the blacklist so we can embed media.
  if (($pos = array_search('iframe', $blacklist)) !== FALSE) {
    unset($blacklist[$pos]);
  }
}
?>
robertom’s picture

I like the possibilities given by #4. Thanks

drurian’s picture

please commit this patch

solipsist’s picture

Please commit this patch. Thanks.

heddn’s picture

+1 on RTBC

Marty2081’s picture

+1 for committing the patch from #4.

bropp’s picture

+1 on RTBC

eugene.ilyin’s picture

Patch #4 helped me. Thanks, please commit it.

heylookalive’s picture

This issue has been going for three years plus now, I'll volunteer to do a small amount of maintenance on the issue queue to get this in. I'd encourage others following this issue to volunteer the same.

eloivaque’s picture

Work for me the patch #4 and the hook #16

vistree’s picture

Works also for me: patch in #4 and example hook in #16
Can this be commited?

  • stefan.r committed 4848d30 on authored by azinck
    Issue #1783966 by azinck: Allow altering of blacklist to enable support...
stefan.r’s picture

Status: Reviewed & tested by the community » Fixed

A lot of people want this and are already running this patch, so let's commit it!
https://www.drupal.org/commitlog/commit/7716/4848d306a2f7526f7eeaf22edb9...

Thanks all!

stefan.r’s picture

Follow-up opened for supporting Youtube iframes: #2663486: Allow for iframes from a whitelisted list of domains

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

gisle’s picture

Thanks for the patch in #4. It is now being used by Src whitelist.