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

Injecting classnames into code blocks #699

Closed
xPaw opened this issue Mar 17, 2019 · 8 comments
Closed

Injecting classnames into code blocks #699

xPaw opened this issue Mar 17, 2019 · 8 comments

Comments

@xPaw
Copy link

xPaw commented Mar 17, 2019

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:

$Element['attributes'] = array('class' => "language-$infostring");

@aidantwoods
Copy link
Collaborator

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
Copy link
Author

xPaw commented Mar 17, 2019

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
Copy link
Collaborator

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

@aidantwoods
Copy link
Collaborator

1.7.2 and 1.8.0-beta-6 now released.

@dregad
Copy link

dregad commented Apr 4, 2019

@aidantwoods wouldn't this require a CVE ?

@aidantwoods
Copy link
Collaborator

Looks like the DWF, who I requested a CVE via in #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 added a commit that referenced this issue Apr 6, 2019
aidantwoods added a commit to aidantwoods/parsedown that referenced this issue Apr 6, 2019
@aidantwoods
Copy link
Collaborator

CVE-2019-10905

aidantwoods added a commit that referenced this issue Apr 6, 2019
aidantwoods added a commit to aidantwoods/parsedown that referenced this issue Apr 7, 2019
@aidantwoods
Copy link
Collaborator

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 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants