SpongePowered / Ore

Repository software for Sponge plugins and Forge mods
https://ore.spongepowered.org/
MIT License
79 stars 25 forks source link

Improve markdown HTML support for plugin page #1052

Open me4502 opened 2 years ago

me4502 commented 2 years ago

Is your feature request related to a problem? Please describe. Ore currently has severely limited Markdown support for plugin page description bodies, making it difficult to produce a richer experience. This also limits the ability to share pages with other sites for cross-platform mods, such as on CurseForge.

When testing this, a vast majority of tags are entirely ignored and kept as plaintext, with div tags creating inline grey boxes on the page.

Describe the solution you'd like Full support for Markdown with the common HTML tags allowed in many markdown variants. See the file in additional context for examples that are not supported.

Describe alternatives you've considered (optional) N/A

Additional context This is an example of a page that would ideally work on Ore, as-is this works fine on CurseForge and BukkitDev. https://gist.github.com/me4502/3f65a37e638755f69087857f0eaf188a

Katrix commented 2 years ago

If we are to do this, we need to make sure to do it in a safe way that prevents phising attempts and XSS. Additionally, I am unsure if Flexmark supports more fine grained control of this. Currently we use SUPPRESS_HTML.

I'll leave this open, but unless someone has some easy to do solution, I don't ever see myself working on this (I have too little time to work on Ore in general nowadays, but this especially seems like a lot of work to get done correctly.)

me4502 commented 2 years ago

As far as I can tell, script tags are not parsed so that should prevent XSS, I don't really know of any Markdown library that allows XSS outside of bugs tbh.

From what I can tell, purely disabling SUPPRESS_HTML would solve this. I cannot think of a situation where a phishing attempt would be allowed by HTML that isn't already allowed by Markdown (links with different URLs displayed etc)

Katrix commented 2 years ago

More complex than that. For example onload in an img tag does not use script but still an XSS vulnerability. Disabling SUPPRESS_HTML would allow unrestricted access to any and all HTML. That is opening up a security hole I do not want to touch. A form for example, that pretends to be a part of the website. Say you want to delete a specific user's project, maybe some drama happened. Just fool the owner to navigate to some page you control with this thing.

<form action="/CompetitorUser/Competition/manage/delete" method="POST">
    <input type="submit" name="delete" value="Some alluring offer" class="btn btn-danger">
</form>

In this case it would not be a hard delete luckily, but it shows the kind of things we need to be VERY careful of IF we do this.

Katrix commented 2 years ago

Another thing to consider too is that the forums need to accept it as well. With the sample you supplied, as long as I unindent it (this proves an interesting bit all by itself TBH), then it works, but it doesn't use much of the formatting either. Only what is already possible with Markdown from what I can see.