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

[CRITICAL BUG] Inject any html/css/js by using KaTeX #1160

Closed
mirik-k opened this issue Feb 15, 2018 · 20 comments
Closed

[CRITICAL BUG] Inject any html/css/js by using KaTeX #1160

mirik-k opened this issue Feb 15, 2018 · 20 comments

Comments

@mirik-k
Copy link

mirik-k commented Feb 15, 2018

Please contact me to arsen.stahanov@gmail.com

@kevinbarabash
Copy link
Member

@ArStah would you be willing to submit this using hackerone.com?

@mirik-k
Copy link
Author

mirik-k commented Feb 15, 2018

@kevinbarabash i can't find your organization there

@kevinbarabash
Copy link
Member

@ArStah it should be listed as Khan Academy.

@mirik-k
Copy link
Author

mirik-k commented Feb 15, 2018

Look here what happens. Maybe it's not KaTeX issue, but i think you must check it

@kevinbarabash
Copy link
Member

kevinbarabash commented Feb 15, 2018

This is appears to be a gitter issue. It looks like they aren't properly sanitizing math input from users.

@edemaine
Copy link
Member

Hard to tell without more information, but I imagine if there's any input to katex.render/katex.renderToString that can output a <script> tag, that'd be a bug in KaTeX. (Of course, gitter also shouldn't be relying on this, and should be sanitizing the output of KaTeX too... I certainly do this in my own project, though it'd be helpful to have guidelines for what to whitelist for KaTeX to work.) @kevinbarabash Are you suggesting that's not the case, and it's some other workaround that avoids calling KaTeX?

@kevinbarabash
Copy link
Member

The KaTeX parser fails on the given input, e.g.

KaTeX parse error: Expected 'EOF', got '\<' at position 3:  \<̲iframe src="ja:  \

@pascalwhoop
Copy link

Yes I also believe it shouldn't be positioned in KaTeX although it could be "good" to help people who add this library to help themselves by sanitizing against any html inside of KaTeX. Or is there any use case where people have html>KaTeX>html?

@kevinbarabash
Copy link
Member

@pascalwhoop could you expand on what you mean by html>KaTeX>html?

@pascalwhoop
Copy link

pascalwhoop commented Feb 15, 2018

Ah sorry. Usually you nest KaTeX code inside of html right? So let's say we use

<p>$$ f(x) = y^2 $$</p>

is there any reason why we should have it be something like

<p>$$ f(x) = y^2 <em>\text{chickens}</em> $$</p>

If not, then forbidding any html inside the string that is passed to katex.render could help developers help themselves. A bit. Because it helps avoid breaking out of the <pre><code> boxes

@kevinbarabash
Copy link
Member

We already "forbid" HTML inside the string we pass to katex.render b/c it's not valid LaTeX`.

@pascalwhoop
Copy link

Okay then how exactly does it occur, that the HTML breaks out? I guess you forbid it, throw an error and the string is output to the UI of the client, which then is in fact our HTML that got injected?

@kevinbarabash
Copy link
Member

KaTeX autorender extension only looks at the $$...$$ that are on the page. If they contain malicious code, once they've been added to the page it's too late.

@pascalwhoop
Copy link

Yes and no.
Gitter probably sanitizes their posts and ensures there is no html in there. But for some reason they exclude the contents of KaTeX and don't turn < into < . Probably because they don't want to mess with the code that's intended to be interpreted by KaTeX.

The error thrown by KaTeX parser then is also not sanitized. The error is thrown on the client side and there is no client side wrapping of the error, sanitizing it. But you could sanitize the Error you throw so users don't have to.

@edemaine
Copy link
Member

edemaine commented Feb 15, 2018

I would hope/expect Gitter isn't using the autorender extension, but rather calling katex.renderToString before rendering the LaTeX source to DOM. Can anyone confirm/deny this?

@pascalwhoop I don't see why the error needs to be sanitized by KaTeX. No KaTeX code will render the error to HTML. At worst, it is printed as a console message.

I definitely don't understand how the leakage of HTML is happening. Feel free to email me details so I can investigate.

@pascalwhoop
Copy link

@edemaine has been contacted by the Gitter team. Gitter renders the KaTeX error as unsanitized HTML, which causes the trouble. I guess to give the user some feedback of what's not working.
I vote for close.
thanks for the triage

@mirik-k
Copy link
Author

mirik-k commented Feb 15, 2018

@pascalwhoop 👍

@edemaine
Copy link
Member

Cool, glad we could help track it down. To help avoid this in the future, I've added a paragraph to the README talking about what's safe and what isn't in PR #1161. Reading it over and critiquing would be appreciated!

@MadLittleMods
Copy link

@edemaine Thanks for the email response earlier ❤️

The situation has been remedied and there is a blog post here, http://blog.gitter.im/2018/02/16/gitter-xss-cryptocoin-mining-security-issue-notification/

@mirik-k
Copy link
Author

mirik-k commented Feb 16, 2018

@MadLittleMods great, thanks for quick reaction 👍

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

No branches or pull requests

5 participants