[Bug] Markdown

[TimerWolf]

• Gitea version: 1.10.0
• Git version: 2.17.1
• Operating system: Linux / Ubuntu 18.04.3 LTS / Bionic
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
  • Not relevant
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (https://try.gitea.io/TimerWolf/MarkdownBug/src/branch/master/Markdown.md)
  • No
  • Not relevant
• Log gist:
  • Not relevant

Description

I think that this and the broken feature is called markdown, and if you use it this way as i do under this text it will break, for some reason it don't here at github :)

If the link provided not working, just use the code that i used here!

Example

#	Status	Hover
1	Works	Here
2	Broken	Here

[jolheiser]

The markdown parser we use must not allow > in link titles. (< also aren't allowed, nor are most symbols from what I could tell)
My best guess is it's to block users from breaking out of the rendered HTML tag and insert potentially malicious elements.

[zeripath]

Have you tried & g t ; ?

[lafriks]

To be honest I don't see difference between github and gitea rendered tables 🤔

[TimerWolf]

The markdown parser we use must not allow > in link titles. (< also aren't allowed, nor are most symbols from what I could tell)
My best guess is it's to block users from breaking out of the rendered HTML tag and insert potentially malicious elements.

Maybe that's the case but why would it be allowed here on github then?

Have you tried & g t ; ?

That did not work ether, but it works here on github as well.

To be honest I don't see difference between github and gitea rendered tables 🤔

The broken one don´t show you the tooltip in "try.gitea.io" when you hover over it.

[jolheiser]

GH uses a different parser than us, more than likely. I haven't had a chance to look for a possible config option, but my guess is we would need to extend our parser for this to work.

[guillep2k]

It took me a while to understand the problem 😁. For reference:

GitHub, mouse hover on first item:

GitHub, mouse hover on second item:

Gitea, mouse hover on first item:

Gitea, mouse hover on second item:

@TimerWolf since we can't access the exact markdown code you've used, would you paste it here in a code block?

    ```
    ### Markdown code
    [text](url)
    ```

[jolheiser]

@guillep2k

[Works](https://: "1, 2 ,3, 4, 5")
[Broken](https://: "1<  2 -> 3 -> 4 -> 5")

Essentially this is exploiting link titles for hovering purposes.
I'm trying to look through goldmark to see where this is processed, but it's admittedly slow going at the moment.

[guillep2k]

No luck. The parser won't even take ＜ (Unicode U+FF1C - FULLWIDTH LESS-THAN SIGN). As far as I can tell, no symbol outside a small subset will be accepted. *, $ and # are rejected, accented characters (á) and comma (,) seem OK, but colon, semicolon and ampersand are out of the question.

[jolheiser]

This looks like an upstream bug, possibly?
It should be allowed per CommonMark spec (Example 502).

[TimerWolf]

@guillep2k: thanks for straight that things out for me, for some reason when i do stuff it always end up that people don't understand what i mean.

I don't know why as i was quite clear to me as i did change the code to "hover -> here" and also actually have the "status" indicator, but thanks again and sorry if anyone if it was a bit unclear.

The code is back to it's original i was never suppose to change not sure why i accepted that change, it was only suppose to be a preview test that @zeripath suggested...

@jolheiser: i like to use the hover for this purpose when it's a lot of text that should fit in and in the same time not break the table, like descriptions.

[mrsdizzie]

This comes from default bluemonday policy and not goldmark:

gitea/vendor/github.com/microcosm-cc/bluemonday/helpers.go

Lines 109 to 112 in dc822d5

	// Paragraph of text in an attribute such as *.'title', img.alt, etc
	// https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes#attr-title
	// Note that we are not allowing chars that could close tags like '>'
	Paragraph = regexp.MustCompile(`^[\p{L}\p{N}\s\-_',\[\]!\./\\\(\)]*$`)

Then enabled for title here:

gitea/vendor/github.com/microcosm-cc/bluemonday/helpers.go

Lines 162 to 163 in dc822d5

	// "title" is permitted as it improves accessibility.
	p.AllowAttrs("title").Matching(Paragraph).Globally()

If we did want to overide it and allow a closing tag for link titles we could use this in https://github.com/go-gitea/gitea/blob/master/modules/markup/sanitizer.go,

sanitizer.policy.AllowAttrs("title").Matching(regexp.MustCompile(`^[\p{L}\p{N}\s\-_',\[\]!\./\\\(\)\>]*$`)).OnElements("a")

Which in testing would output:

<td>Broken</td>
<td><a href="https://:" title="1 -&gt; 2 -&gt; 3 -&gt; 4 -&gt; 5">How</a></td>

Which seems fine in that example. I believe. You can see it currently doesn't allow other characters as well (pretty much anything that isn't a word number and those few punctuations listed)

[jolheiser]

Shoot, I thought I tested it well enough locally. Good catch @mrsdizzie!

[mrsdizzie]

@jolheiser Does that seem like something we should change you think?

[jolheiser]

It seems that would solve this particular issue, but I'm not sure if there would be other unintended effects. Based on the comment in the bluemonday excerpt, it seems like > was left out intentionally.

If you test with that sanitizer, can you escape titles using raw html?

[mrsdizzie]

Hmm seems fine in my probably limited testing

[jolheiser]

We switcht from blackfriday to gildmark, so this may also fixed this issue

As mentioned above, this is because of our sanitizer, not goldmark.

Hmm seems fine in my probably limited testing

@mrsdizzie would you mind opening a PR? We can discuss whether we should include more symbols there?

I'm still cautious because bluemonday specifically mentions blocking >

[mrsdizzie]

@jolheiser opened PR for further discussion

[lafriks]

Fixed by #10527 if not please reopen