rails/rails-html-sanitizer

Rails Sanitize Gem: Frozen Allowed Tags List is Mutated Internally

ayzahamid opened this issue · 5 comments

We have observed a problematic behavior in the latest version of the sanitize gem when using a frozen list for allowed_tags. Specifically, the gem mutates the provided frozen list internally, which leads to a RuntimeError (can't modify frozen Array) or unexpected behavior.
This mutation is unexpected because frozen objects are intended to remain immutable, and the gem should ideally create a copy of the provided list if internal modifications are required.
So, to avoid this problem we had to use .dup to create a shallow copy, which should be happening internally.

@ayzahamid Sorry you're having a problem. I'm not sure I understand, can you provide more information, please? What is the error or exception you're seeing? Can you provide a reproduction so I can diagnose what's happening?

@ayzahamid Sorry you're having a problem. I'm not sure I understand, can you provide more information, please? What is the error or exception you're seeing? Can you provide a reproduction so I can diagnose what's happening?

For rails-html-sanitizer gem version 1.6.1, I am facing this issue where i have a frozen array named ALLOWED_TAGS and i pass that as tags in:

new.sanitize(html, tags: ALLOWED_TAGS, attributes: ALLOWED_ATTRIBUTES)

it mutates the ALLOWED_TAGS internally and i get the below error:
image

@ayzahamid I'm sorry you're having this problem. You're right, in this case we should be duping the list internally, and I'll work on a fix.

In them meantime, you should know that the reason this gem is attempting to modify the allowed tags is because mglyph is unsafe to use. It, along with noscript and malignmark are removed from the allowed tags if they are present, as noted in the GHSAs linked from the CHANGELOG.md file.

If you remove them from your list, you should be fine.

I will get a bugfix release out as soon as I'm able.