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.
Comment | File | Size | Author |
---|---|---|---|
#4 | wysiwyg_filter_alter_blacklist-1783966-4.patch | 533 bytes | azinck |
#1 | wysiwyg_filter-alter-blacklist.patch | 1.45 KB | azinck |
Comments
Comment #1
azinck CreditAttribution: azinck commentedSimple 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.
Comment #2
azinck CreditAttribution: azinck commentedComment #3
drasgardian CreditAttribution: drasgardian commentedI like this approach to allow hooks to alter the blacklist list.
Comment #4
azinck CreditAttribution: azinck commentedI noticed I uploaded a crufty patch with stuff from my environment. Clean patch below.
Comment #5
vinmassaro CreditAttribution: vinmassaro commentedThis 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.
Comment #6
vinmassaro CreditAttribution: vinmassaro commentedNevermind, figured it out:
This works great. Thanks!
Comment #7
amaasbier CreditAttribution: amaasbier commentedI 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.
Comment #8
azinck CreditAttribution: azinck commentedamaasbier: 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.
Comment #9
amaasbier CreditAttribution: amaasbier commentedThanks 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.
Comment #10
TimG1 CreditAttribution: TimG1 commented+1 #4 worked great, thanks for the patch.
-Tim
Comment #11
PierreV CreditAttribution: PierreV commented#9 : I used #4 patch to allow iframes, with this (in wysiwyg_filter.pages.inc, function wysiwyg_filter_filter_wysiwyg_process) :
It's just a quick a dirty way to allow only some medias, but it works.
Comment #12
azinck CreditAttribution: azinck commentedI 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.
Comment #13
maximpodorov CreditAttribution: maximpodorov commentedAny plans to commit the patch? Is really works.
Comment #14
seanBPatch in #4 works perfect! Hope it gets added to the module.
Comment #15
waitnsee CreditAttribution: waitnsee commentedI'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??
Comment #16
maximpodorov CreditAttribution: maximpodorov commentedThir real module adds IFRAME as the acceptable tag for wysiwyg filter configuration:
Comment #17
heylookalive CreditAttribution: heylookalive commented+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.
Comment #18
askibinski CreditAttribution: askibinski commented#4 works like a charm in combination with a custom module.
Comment #19
lordzik CreditAttribution: lordzik commentededit:
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" :)
Comment #20
muschpusch CreditAttribution: muschpusch commented+ 1 for committing!
Comment #21
seanBLooks like we all agree that this is a great fix to allow more flexibility. Is there a reason to not commit this?
Comment #22
marcoscano+1 for commiting
the patch in #4 works as expected
Comment #23
rooby CreditAttribution: rooby commented+1 for this also.
Re #16, you can do things like that without having to loop using array_search() or array_keys().
Comment #24
rooby CreditAttribution: rooby commented#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:
Comment #25
robertom CreditAttribution: robertom commentedI like the possibilities given by #4. Thanks
Comment #26
drurian CreditAttribution: drurian commentedplease commit this patch
Comment #27
solipsist CreditAttribution: solipsist commentedPlease commit this patch. Thanks.
Comment #28
heddn+1 on RTBC
Comment #29
Marty2081 CreditAttribution: Marty2081 commented+1 for committing the patch from #4.
Comment #30
bropp CreditAttribution: bropp commented+1 on RTBC
Comment #31
eugene.ilyin CreditAttribution: eugene.ilyin as a volunteer and at DrupalJedi commentedPatch #4 helped me. Thanks, please commit it.
Comment #32
heylookalive CreditAttribution: heylookalive commentedThis 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.
Comment #33
eloivaqueWork for me the patch #4 and the hook #16
Comment #34
vistree CreditAttribution: vistree commentedWorks also for me: patch in #4 and example hook in #16
Can this be commited?
Comment #36
stefan.r CreditAttribution: stefan.r commentedA 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!
Comment #37
stefan.r CreditAttribution: stefan.r commentedFollow-up opened for supporting Youtube iframes: #2663486: Allow for iframes from a whitelisted list of domains
Comment #39
gisleThanks for the patch in #4. It is now being used by Src whitelist.