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

Don't strip HTML classes/style from custom markup #4935

Closed
2 of 7 tasks
coreagile opened this issue Sep 14, 2018 · 6 comments
Closed
2 of 7 tasks

Don't strip HTML classes/style from custom markup #4935

coreagile opened this issue Sep 14, 2018 · 6 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@coreagile
Copy link

  • Gitea version (or commit ref): 1.5.0
  • Git version: 2.7.4
  • Operating system: Ubuntu 16.04.5 LTS \n \l
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

When using a custom renderer (see below), much of the HTML classes are stripped from the rendered code, removing useful style (like coloring). Can we tell the renderer not to do that?

Here's my config:

[markup.html]
ENABLED         = true
FILE_EXTENSIONS = .html
RENDER_COMMAND  = /usr/local/bin/inline-html
IS_INPUT_FILE   = false

And the contents of /usr/local/bin/inline-html:

#!/bin/bash
[ $# -ge 1 -a -f "$1" ] && input="$1" || input="-"
cat $input | pup 'body' | tail -n+2 | head -n-2

I've verified that the contents of the output of /usr/local/bin/inline-html retains the HTML classes and styles of the original document. However, by the time the markup is inlined into my gitea page, it's all stripped away.
...

Screenshots

This is what a portion of my "README.html" looks like with the current version of Gitea:
image

And this is what it looks like when I view the file directly:
image

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Sep 14, 2018
@nithinphilips
Copy link

This also affects restructuredText rendering with rst2html, which uses pygments to generate syntax highlighted html.

Configuration:

app.ini:

[markup.restructuredText]
ENABLED         = true
FILE_EXTENSIONS = .rst
RENDER_COMMAND = "python3 /usr/local/bin/rst2html.py --leave-comments --syntax-highlight=short"
IS_INPUT_FILE   = false

I placed pygments.css (generated by running pygmentize -f html -S colorful) in <gitea>/custom/public directory and added a header.tmpl in <gitea>/custom/templates/custom with the following content:

<link rel="stylesheet" href="/pygments.css">

Output from running the command directly:

<pre class="code json literal-block">
<span class="s2">&quot;attributes&quot;</span> <span class="err">:</span> <span class="p">[</span> <span class="p">{</span>
<span class="p">}</span> <span class="p">]</span>
</pre>

Same content in the Gitea page:

<pre>
<span>&quot;attributes&quot;</span> <span>:</span> <span>[</span> <span>{</span>
<span>}</span> <span]</span>
</pre>

@lunny
Copy link
Member

lunny commented Oct 9, 2018

The class attributes had been removed by this rule for security, see https://github.com/go-gitea/gitea/blob/master/modules/markup/sanitizer.go#L33 . Please send a PR to change that.

@nithinphilips
Copy link

nithinphilips commented Oct 9, 2018

I have produced a local build that solves my issue by adding this line to the sanitizer:

sanitizer.policy.AllowAttrs("class").OnElements("pre", "span")

This will allow class attributes on any pre and span elements. I could not find a way to restrict this with bluemonday based on parent-child relation (ie. only allow class on span within pre class="code"), which would be a less disruptive change.

To solve @coreagile's original issue, class attribute has to be allowed globally + allow style tags. However untrusted CSS could enable code execution/vulnerable font downloads etc. and probably not a good idea. A workaround would be to allow class attributes (it doesn't look like class attributes can have any executable code) and then specify a trusted css via header.tmpl template.

Any thoughts?

@lunny
Copy link
Member

lunny commented Jul 7, 2020

#9075 may fixed this problem.

@zeripath
Copy link
Contributor

zeripath commented Jul 7, 2020

I think if we just auto-prefix classes within user generated content with user-content- (excepting the few we know are safe) then this problem can be pushed on to end users who can then decide what to do with with this.

The issue with just allowing class straight through is that some js uses class as selectors.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jul 5, 2022

Now there are new render modes: no-sanitizer and iframe.

;; How the content will be rendered.
;; * sanitized: Sanitize the content and render it inside current page, default to only allow a few HTML tags and attributes. Customized sanitizer rules can be defined in [markup.sanitizer.*] .
;; * no-sanitizer: Disable the sanitizer and render the content inside current page. It's **insecure** and may lead to XSS attack if the content contains malicious code.
;; * iframe: Render the content in a separate standalone page and embed it into current page by iframe. The iframe is in sandbox mode with same-origin disabled, and the JS code are safely isolated from parent page.
;RENDER_CONTENT_MODE=sanitized

@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

5 participants