Upgrade jquery from v1.7.1 to v1.9.0 to fix security vulnerability

[floehopper]

Description

This is a lot less ambitious than #1294 and so hopefully shouldn't
introduce any problems. The idea is to do just enough to address the
CVE-2017-16011 security vulnerability.

I downloaded the jquery JS from here.

Completed Tasks

• I have read the Contributing Guide.
• The pull request is complete (implemented / written).
• Git commits have been cleaned up (squash WIP / revert commits).
• I wrote tests and ran bundle exec rake locally (if code is attached to PR).

[coveralls]

Coverage remained the same at 93.439% when pulling 5cb6e69 on freerange:upgrade-jquery-from-v1.7.1-to-v1.9.0 into 2b16190 on lsegal:main.

[floehopper]

Bump! Any chance of getting this merged? Thanks.

[floehopper]

In case anyone else has run into this problem, I've worked around it temporarily here: freerange/mocha@211098a.

[floehopper]

I've only just noticed that jquery v1.9.0 introduces a problem with the "toggle source" links. I've downgraded jquery to v1.8.3 in my own project and that seems to fix the problem. I'm going to continue to investigate whether there's a way to upgrade back to v1.9.0, because that's the version that fixes the security vulnerability.

[floehopper]

I've now pushed another commit which fixes the fact that the version of the jquery toggle function which takes 2 functions as arguments was removed in jquery v1.9.0. See the commit note for more details: e6d2492.

[floehopper]

@lsegal:

Thanks for your response on Twitter.

From this tweet:

Without a full regression on YARD's many templates it would be hard to merge this as is. AFAIK there are many breaking changes between 1.7 and 1.9 that can affect downstream users, and the XSS vuln in question is kind of moot given that you can already inject JS via templates.

OK. In case it helps, I used the jQuery Migrate plugin to work out why the "view source" functionality wasn't working and the jQuery.fn.toggle(handler, handler...) deprecation warning was the only deprecation warning I saw on any of the pages I looked at. I wonder if it might be sufficient to use this tool more to more thoroughly check all page types...? That way at least we've moved to a slightly more recent version of jQuery.

And from this tweet:

The cleaner way to insulate from this issue would be to sanitize wherever selectors are used on user input, which seems to only happen on TOCs:

yard/templates/default/fulldoc/html/js/app.js

Line 184 in ca59056

var proposedId = $(this).attr('toc-id');

Sounds sensible, but I'm not sure whether I'll have time to work on that any time soon.

[lsegal]

I wonder if it might be sufficient to use this tool more to more thoroughly check all page types...?

YARD can generate guide based templates too (via yard doc -t guide which generates only your README/extra-files), so you would have to check that template. The bigger issue is that others may have extended the default template and might be relying on more jquery features, so this would generally be seen as an unfortunate breaking change that affects downstream consumers.

It's more unfortunate that (a) YARD is a little outdated by using jQuery in general, and (b) jQuery has decided to make breaking changes in the middle of major versions, meaning in order to get this fix we'd need to incorporate breaking changes. That said, as mentioned above (and here) the actual impact of this jQuery vulnerability might be pretty low for YARD, a tool that already allows devs to put whatever they want inside a generated HTML document. It also may be that this vulnerability doesn't affect YARD at all. I cannot guarantee this, but it doesn't look like YARD generates selectors from user inputs in any place other than TOC id/toc-id attributes, which tend to be generated by other sanitizing tools (markdown, etc).

This probably affects sites that serve output a little more (like rubydoc.info), but even there we don't necessarily block people from customizing frontend, or at least, the goal is to allow developers to affect HTML up-to-and-including adding extra scripting into a page, so this might be something downstream consumers need to take on. Quite frankly, if XSS is a concern for user generated content, those who use YARD in this way should realize that the tool allows for quite liberal generation on content, including, for example, rendering a README.html file in your repo as an unsanitized HTML document in the generated output. Yes you read this right, any repo can contain a .html document that is rendered as is, scripts and all by simply adding it as an extra file. YARD is a generalized content generation tool, and as such, downstream users who are rendering arbitrary user-generated content need to be mindful of that fact and sanitize user input before it goes into the tool. (PS: you can use -m html to convert all your docstrings to bare HTML too. Fun stuff!)

Until this is patched for affected downstream usage, interested users could replace the jquery.js that is vendored in YARD with an updated version + relevant toggle patches as a post-step after running yard doc. Note that this doesn't solve the rendering .html extra files, of course, that is something downstream users would also have to address if they are not doing so.

[floehopper]

@lsegal Thanks for explaining. Sorry not to respond sooner. I'm going to close this PR.