search

coffeedoc / codo

CoffeeScript API documentation generator. It's like YARD but for CoffeeScript!
Other
625 stars 92 forks source link

Codo allows <script> tags to be executed in the docs #166

Closed jvennix-r7 closed 10 years ago

jvennix-r7 commented 10 years ago

This is problematic since most users will view the docs from a file:// URL, so the script has access to the full filesystem on some browsers. Not sure if this is part of a feature (embedding HTML in your docstrings), but some sort of sanitization would be nice to prevent scripts from running here.

Reproduce: Run codo on a method documentation string containing <script>alert(1)</script>.

inossidabile commented 10 years ago

You normally run Codo on your own projects and then share HTML. You can always add malicious code after the generation. At the same time this can't happen accidentally. There's no point in such checks - they prevent non-existing risk.

jvennix-r7 commented 10 years ago

I feel like that is glossing over some major use cases... For example http://coffeedoc.info/ uses codo to generate and host docs in an automated way, and many companies have a similar internal set up. I am guessing (hoping) that coffeedoc.info does some sort of sanitization of its own, why not just make that part of codo? Additionally, if I am regenerating docs while reviewing a PR from the community, I dont want to manually read every single comment for the possibility of XSS. Yard recognizes these use-cases and provides a --safe flag to prevent plugins from being auto run, and seems receptive to the idea of having --safe mode sanitize javascript. Any chance you'll give this feature a second look? I am happy to help implement. Otherwise I'll stop bothering you and just put this in my own fork :)

inossidabile commented 10 years ago

In case of automated generation you are not running it from file:// anymore. And it's just a static HTML so no script injection makes sense here. Do not overcomplicate things my friend :). I'll happily merge this paranoid PR if you are up to make it on your own though. I simply can't come up with ANY attack vector based on this and can't see any reason to spend time on that.

jvennix-r7 commented 10 years ago

Okay, thanks :) Mostly it is unexpected/unnecessary and I would rather have some option to disable it.