Skip to content
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.

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

Remove excess Div wrappers #4528

Merged
merged 5 commits into from
Oct 9, 2021
Merged

Remove excess Div wrappers #4528

merged 5 commits into from
Oct 9, 2021

Conversation

broxen
Copy link
Contributor

@broxen broxen commented Oct 1, 2021

As noted in #4524 there are an excess of div wrappers in generated page html.

This PR removes this code section because it seems that standalone text is already wrapped in paragraph elements, making this superfluous and simultaneously correcting the issue of excess div wrappers.

Closes #4524

It seems like standalone text is already wrapped in paragraph elements, so this code seems superfluous. Additionally, it adds div wrappers at every line break as described in requarks#4524
@NGPixel
Copy link
Member

NGPixel commented Oct 2, 2021

These excess divs are actually there to protect against XSS at the root level in non-div elements. There's probably a more elegant solution but this was a quick way to ensure content is sanitized properly further down the rendering pipeline.

This change skips newlines and returns to focus on unbounded text only.
@broxen
Copy link
Contributor Author

broxen commented Oct 2, 2021

to protect against XSS at the root level

Ah, I see. I've pushed a proposed change to this so that it ignores "empty" newlines and returns, but still captures potentially dangerous text. I don't know if it's any more elegant, but it works 😄

Tested with the following text in a new page:

<h3>XSS Attack</h3>
<p>Some text here</p> {{constructor.constructor('alert("Wiki.JS Stored XSS")')()&#x7d&#x7d

a
b\n
[][}{}{}<<<<<>>>>>

@NGPixel NGPixel merged commit 12aef93 into requarks:dev Oct 9, 2021
@broxen broxen deleted the remove-divs branch October 10, 2021 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Superfluous wrapper divs in generated pages affects css
2 participants