Skip to content

[HtmlSanitizer] Allowed height but disallowed width #50153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
cedric-anne opened this issue Apr 25, 2023 · 3 comments
Closed

[HtmlSanitizer] Allowed height but disallowed width #50153

cedric-anne opened this issue Apr 25, 2023 · 3 comments
Labels
Bug HtmlSanitizer Status: Needs Review

Comments

@cedric-anne
Copy link
Contributor

Symfony version(s) affected

6.2.7

Description

The "safe" configuration for HtmlSanitizer (using (new HtmlSanitizerConfig())->allowSafeElements() allows the height attribute, but blocks the width attribute. IMHO, both should be allowed, or both should be blocked.

https://github.com/symfony/html-sanitizer/blob/eae9b0a9ad7a2ed1963f819547d59ff99ad9e0fd/Reference/W3CReference.php#L274

https://github.com/symfony/html-sanitizer/blob/eae9b0a9ad7a2ed1963f819547d59ff99ad9e0fd/Reference/W3CReference.php#L397

How to reproduce

<?php

use Symfony\Component\HtmlSanitizer\HtmlSanitizer;
use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig;

$config = (new HtmlSanitizerConfig())->allowSafeElements();
echo (new HtmlSanitizer($config))->sanitize('<table height=100 width=0 align="left" cellspacing=10><tr><td>Test</td></tr></table>');

// <table height="100" align="left" cellspacing="10"><tr><td>Test</td></tr></table>

Possible Solution

No response

Additional Context

No response
@stof
Copy link
Member

stof commented Apr 26, 2023

/cc @tgalopin

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@nicolas-grekas
Copy link
Member

A PR would help on this topic.

@carsonbot carsonbot removed the Stalled label Oct 27, 2023
@cedric-anne cedric-anne mentioned this issue Oct 27, 2023
Merged
@fabpot fabpot closed this as completed Oct 27, 2023
fabpot added a commit that referenced this issue Oct 27, 2023
@fabpot
minor #52324 [HtmlSanitizer] Consider width attribute as safe (cedr…
b691eba 
…ic-anne)

This PR was merged into the 6.3 branch.

Discussion
----------

[HtmlSanitizer] Consider `width` attribute as safe

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #50153
| License       | MIT

Consider the HTML attribute `width` to be safe, as attribute `height` already is.

Commits
-------

827bd6a [HtmlSanitizer] Consider `width` attribute as safe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HtmlSanitizer Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @nicolas-grekas @stof @carsonbot @cedric-anne