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

Self XSS #727

Closed
Nahiiko opened this issue Dec 28, 2021 · 3 comments
Closed

Self XSS #727

Nahiiko opened this issue Dec 28, 2021 · 3 comments
Labels
Bug resolved if issue is resolved, it will be open until merge with master

Comments

@Nahiiko
Copy link

Nahiiko commented Dec 28, 2021

Self XSS is possible by typing something like "onclick="alert(1);"

I think it's because data-text doesn't contain sanitized characters.
Don't think it's critical but there you go regardless :)

@jcubic
Copy link
Owner

jcubic commented Dec 29, 2021

Can you create a PoC for this? Without reproduction it's like saying "your library is broken" without saying anything else.

Please show example, how this lead to XSS, if it's true is more problematic than you think.
I've already found some XSS long ago and reported it to NPM, because the developer (User) of the library could unintentionally create reflected XSS if he was not careful. That's why I've disabled some options by default.

@jcubic
Copy link
Owner

jcubic commented Dec 29, 2021

Ok, I was able to reproduce, I was testing in Terminal JavaScript demo that is not affected.

Thanks for the report.

@jcubic
Copy link
Owner

jcubic commented Dec 30, 2021

I think that I've forget in one place to escape " with " it's happen only when formatting is enabled (the XSS is not present if so). But outside of formatting the code is not escaped. Probably the code for data-text attribute was added later so data-text attribute is always present, but escaping was not added to this new code.

And just FYI: to prevent the XSS you can use this code:

$.terminal.new_formatter([/([\s\S]+)/g, '[[;;]$1]']);

that wraps whole text into empty formatting, it's no op but it will trigger the code that do escape the " with ".

@jcubic jcubic added Bug resolved if issue is resolved, it will be open until merge with master labels Dec 30, 2021
@jcubic jcubic closed this as completed Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug resolved if issue is resolved, it will be open until merge with master
Projects
None yet
Development

No branches or pull requests

2 participants