search

erusev / parsedown

Better Markdown Parser in PHP
https://parsedown.org
MIT License
14.74k stars 1.12k forks source link

Injecting classnames into code blocks #699

Closed xPaw closed 5 years ago

xPaw commented 5 years ago

Tested on 1.8.0-beta-5

In safe mode with html markup disabled, it is possible to insert any classname into a code block like this:

```js any-class-name with spaces code ```

renders as: <code class="language-js any-class-name with spaces">code</code>

infostring needs some cleanup here: https://github.com/erusev/parsedown/blob/21c8c792de54c91bb4ab42aae033ffe09251eb43/Parsedown.php#L473

aidantwoods commented 5 years ago

Taking a look at the CommonMark spec, it looks like it is recommended that this should render as class="language-js".

The first word of the info string is typically used to specify the language of the code sample, and rendered in the class attribute of the code tag. However, this spec does not mandate any particular treatment of the info string.

Do you have a particular attack in mind here, other than minor defacement? Best (non-contrived) thing I can come up with is making styling look a little funny (by attaching specific defined classes) to a code block.

If we're going contrived, then perhaps if a script already on the page would execute the contents of any element with an execute-me class 🤷‍♂️

xPaw commented 5 years ago

There's certainly things that could cause some breakage on the page, for example styles that make elements fixed or sticky. Scripts looking for particular classes, as you mentioned, could cause unknown problem as well.

The fix here could be simply putting the word before first space into the class and dropping the rest?

aidantwoods commented 5 years ago

Could you confirm: #700 and #701 should fix this in the latest stable branch and latest pre-release?

aidantwoods commented 5 years ago

1.7.2 and 1.8.0-beta-6 now released.

dregad commented 5 years ago

@aidantwoods wouldn't this require a CVE ?

aidantwoods commented 5 years ago

Looks like the DWF, who I requested a CVE via in https://github.com/erusev/parsedown/issues/590, has shutdown, which is a shame 😞.

I think I'd now have to go to MITRE directly to request a CVE.

There's somewhat unclear impact with this one, since it'll require abusing existing functionality on the page but I'll do my best to describe potential impact and request one anyway—we'll see what they say :)

aidantwoods commented 5 years ago

CVE-2019-10905

aidantwoods commented 5 years ago

By the way—and sorry I didn't say this sooner—thank you @xPaw for taking the time to report this and review the fix, it's very much appreciated 😄