[Bug] Markdown #4876

@jolheiser commented on GitHub (Feb 18, 2020):

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.

@jolheiser commented on GitHub (Feb 18, 2020): 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.

GiteaMirror commented

@zeripath commented on GitHub (Feb 18, 2020):

Have you tried & g t ; ?

@zeripath commented on GitHub (Feb 18, 2020): Have you tried & g t ; ?

GiteaMirror commented

@lafriks commented on GitHub (Feb 18, 2020):

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

@lafriks commented on GitHub (Feb 18, 2020): To be honest I don't see difference between github and gitea rendered tables :thinking:

GiteaMirror commented

@TimerWolf commented on GitHub (Feb 18, 2020):

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.

@TimerWolf commented on GitHub (Feb 18, 2020): > > > 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.

GiteaMirror commented

@jolheiser commented on GitHub (Feb 18, 2020):

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.

@jolheiser commented on GitHub (Feb 18, 2020): 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.

GiteaMirror commented

@guillep2k commented on GitHub (Feb 18, 2020):

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)
    ```

@guillep2k commented on GitHub (Feb 18, 2020): It took me a while to understand the problem 😁. For reference: #### GitHub, mouse hover on first item: ![image](https://user-images.githubusercontent.com/18600385/74762260-3fdae380-525c-11ea-991b-b157b9ac514d.png) #### GitHub, mouse hover on second item: ![image](https://user-images.githubusercontent.com/18600385/74762304-55500d80-525c-11ea-9503-6af49a103fdc.png) #### Gitea, mouse hover on first item: ![image](https://user-images.githubusercontent.com/18600385/74762341-6bf66480-525c-11ea-85f2-9b535e39a0d6.png) #### Gitea, mouse hover on second item: ![image](https://user-images.githubusercontent.com/18600385/74762372-7add1700-525c-11ea-89e6-5b7f8a5f70d4.png) @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) ``` ```

GiteaMirror commented

@jolheiser commented on GitHub (Feb 18, 2020):

@guillep2k

[Works](https://: "1, 2 ,3, 4, 5")
[Broken](https://: "1&lt;  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.

@jolheiser commented on GitHub (Feb 18, 2020): @guillep2k ```markdown [Works](https://: "1, 2 ,3, 4, 5") [Broken](https://: "1< 2 -> 3 -> 4 -> 5") ``` Essentially this is exploiting [link titles](https://www.markdownguide.org/basic-syntax#adding-titles "Adding 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.

GiteaMirror commented

@guillep2k commented on GitHub (Feb 18, 2020):

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.

@guillep2k commented on GitHub (Feb 18, 2020): 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.

GiteaMirror commented

@jolheiser commented on GitHub (Feb 18, 2020):

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

@jolheiser commented on GitHub (Feb 18, 2020): This looks like an upstream bug, possibly? It should be allowed per [CommonMark spec (Example 502)](https://spec.commonmark.org/0.29/#example-502).

GiteaMirror commented

@TimerWolf commented on GitHub (Feb 18, 2020):

@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.

@TimerWolf commented on GitHub (Feb 18, 2020): @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.

GiteaMirror commented

dc822d5291/vendor/github.com/microcosm-cc/bluemonday/helpers.go (L109-L112)

@mrsdizzie commented on GitHub (Feb 19, 2020):

This comes from default bluemonday policy and not goldmark:

Then enabled for title here:

dc822d5291/vendor/github.com/microcosm-cc/bluemonday/helpers.go (L162-L163)

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)

@mrsdizzie commented on GitHub (Feb 19, 2020): This comes from default bluemonday policy and not goldmark: https://github.com/go-gitea/gitea/blob/dc822d5291c208bc21fe1312de2a558468e8eebc/vendor/github.com/microcosm-cc/bluemonday/helpers.go#L109-L112 Then enabled for title here: https://github.com/go-gitea/gitea/blob/dc822d5291c208bc21fe1312de2a558468e8eebc/vendor/github.com/microcosm-cc/bluemonday/helpers.go#L162-L163 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 -> 2 -> 3 -> 4 -> 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)

GiteaMirror commented

@jolheiser commented on GitHub (Feb 19, 2020):

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

@jolheiser commented on GitHub (Feb 19, 2020): Shoot, I thought I tested it well enough locally. Good catch @mrsdizzie!

GiteaMirror commented

@mrsdizzie commented on GitHub (Feb 19, 2020):

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

@mrsdizzie commented on GitHub (Feb 19, 2020): @jolheiser Does that seem like something we should change you think?

GiteaMirror commented

@jolheiser commented on GitHub (Feb 19, 2020):

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?

@jolheiser commented on GitHub (Feb 19, 2020): 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?

GiteaMirror commented

@mrsdizzie commented on GitHub (Feb 19, 2020):

Hmm seems fine in my probably limited testing

@mrsdizzie commented on GitHub (Feb 19, 2020): Hmm seems fine in my probably limited testing

GiteaMirror commented