apostrophecms / apostrophe

A full-featured, open-source content management framework built with Node.js that empowers organizations by combining in-context editing and headless architecture in a full-stack JS environment.
https://apostrophecms.com
MIT License
4.34k stars 590 forks source link

Security Issues default jQuery 1.11.3 #2167

Closed ToshKoevoets closed 4 years ago

ToshKoevoets commented 4 years ago

The default jQuery that's loaded has a few security issues.

It might be a good idea to start setting the latest jquery 3 as default:

https://snyk.io/vuln/npm:jquery

boutell commented 4 years ago

For sure, but for bc reasons we set jQuery: 3 in our apostrophe-boilerplate project instead, so that people following our documentation are secure by default.

Similar to our decision to force Node 8 or above (because we really have no choice), we could conceivably do this too, but it really would probably break some projects in the wild, and I seem to recall it's debatable as to whether these vulnerabilities are actually relevant in Apostrophe or not. So I would hesitate to cause agita on old projects that already probably know they should upgrade.

abea commented 4 years ago

@boutell Maybe you looked into this when we added the jQuery v3 option. How much lifting do you think it might take to reasonably map jQuery v1 API differences to v3 to only have one, solid version in there?

I'm also not finding in our documentation where we tell people how to set their older projects to v3. I can work on that.

boutell commented 4 years ago

You mean emulate jQuery 1 on jQuery 3? We would have to read through the changelogs for the 2.0 and 3.0 releases and see. I think that could be messy, so it would make more sense to start by evaluating what the actual security issues are with jQuery 1 and if they warrant the effort. And don't forget the issue of which version of jQuery UI to use.

I'd be against a heroic lift here — the proper resolution is for developers to set jQuery: 3, iron out their issues if any, and move on. We could start displaying a warning in dev, that might be the 80/20 fix.

On Wed, Apr 1, 2020 at 9:54 AM Alex Bea notifications@github.com wrote:

@boutell https://github.com/boutell Maybe you looked into this when we added the jQuery v3 option. How much lifting do you think it might take to reasonably map jQuery v1 API differences to v3 to only have one, solid version in there?

I'm also not finding in our documentation where we tell people how to set their older projects to v3. I can work on that.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2167#issuecomment-607263856, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27PETPJTJ4IXUUD3YELRKNBQ7ANCNFSM4LY4IUPQ .

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

ToshKoevoets commented 4 years ago

My suggestion would be to set jQuery 3 as a default and be able turn on 1 in the options.

I have gone trough security issues. They don't seem likely to happen. But it is mainly an issue if people uses the jQuery library in their own code. For instance if they forget the dateType property in an Ajax call, or of they use user input formatted into JSON directly in the merge function.

Practically I'm running into it because pentesters find it a good reason to complain to upper management, whether or not it actually effects the applications that run on ApostropheCMS or not for corporates or governments that require pentesting it would be a returning issue (they only care about checkboxes).

abea commented 4 years ago

I agree that it doesn't look great that our default has these vulnerabilities, even if the boilerplate has a fix via configuration. Hopefully the lift wouldn't be overly heroic (a few days?). It seems at least leaving this as a reported bug to triage. If @ToshKoevoets or someone else wants to contribute to the fix, even better.

ToshKoevoets commented 4 years ago

Codewise it's not very challenging.

But an update doesn't ensure backwards compatibility, since it forces current users to migrate jQuery to 3. For many it will be without issues, but for others some unexpected bugs might happen.

https://jquery.com/upgrade-guide/3.0/

Maybe easier to set it default to jQuery 3 in newly created projects in the CLI? Im assuming it will be solved in Apos 3 anyway?

Example for changing the defaults in apostrophe-assets directly, can make a pr if wanted:

` self.setDefaultStylesheets = function() { // Default stylesheet requirements self.stylesheets = [ // Must have a jQuery UI theme or acceptable substitute for // autocomplete and datepickers to be usable. -Tom { name: (self.options.jQuery === 1) ? 'vendor/jquery-ui' : 'vendor/jquery-ui-3', when: 'always' },

    { name: 'vendor/pikaday', when: 'always' },
    // { name: 'vendor/jquery.Jcrop', when: 'user' }
    { name: 'vendor/cropper', when: 'user' },
    { name: 'vendor/spectrum', when: 'user' }
  ];
};

self.setDefaultScripts = function() {
  // Default browser side script requirements
  // TODO: lots of override options
  self.scripts = [
    // VENDOR DEPENDENCIES

    // Call setImmediate safely on browser side; much faster
    // than setTimeout(fn, 0)
    { name: 'vendor/setImmediate', when: 'always' },
    // For elegant, cross-browser functional-style programming.
    // Yes, we are standardized on lodash 3 due to our own development
    // cycle.
    { name: 'vendor/lodash', when: 'always' },
    // For async code without tears
    { name: 'vendor/async', when: 'always' },
    // For manipulating dates and times
    { name: 'vendor/moment', when: 'always' },
    // For everything DOM-related
    { name: (self.options.jQuery === 1) ? 'vendor/jquery' : 'vendor/jquery-3', when: 'always' },

    // For parsing query parameters browser-side
    { name: 'vendor/jquery-url-parser', when: 'always' },
    // For blueimp uploader, drag and drop reordering of anything
    // & autocomplete
    { name: (self.options.jQuery === 1) ? 'vendor/jquery-ui' : 'vendor/jquery-ui-3', when: 'always' },
    // For the RTE
    { name: 'vendor/jquery-hotkeys', when: 'user' },
    // Graceful fallback for older browsers for jquery fileupload
    { name: 'vendor/jquery.iframe-transport', when: 'user' },
    // Spiffy multiple file upload
    { name: 'vendor/jquery.fileupload', when: 'user' },
    // imaging cropping plugin
    // { name: 'vendor/jquery.Jcrop.min', when: 'user' },
    { name: 'vendor/cropper', when: 'user' },
    // textchange event, detects actual typing activity, not just focus change
    { name: 'vendor/jquery-textchange', when: 'always' },
    // preferred datepicker plugin
    { name: 'vendor/pikaday', when: 'always' },
    // Set, get and delete cookies in browser-side JavaScript
    { name: 'vendor/jquery.cookie', when: 'always' },
    { name: 'vendor/jquery.findSafe', when: 'user' },
    { name: 'vendor/jquery.onSafe', when: 'user' },
    { name: 'vendor/jquery.alter-class', when: 'user' },
    { name: 'vendor/sluggo', when: 'user' },

    // Spectrum is a colorpicker
    { name: 'vendor/spectrum', when: 'user' },

    // Scroll things into view, even if they are in a scrolling
    // container which itself needs to be scrolled into view or
    // whatever, it's pretty great:
    //
    // http://erraticdev.blogspot.com/2011/02/jquery-scroll-into-view-plugin-with.html
    //
    // (Note recent comments, it's actively maintained). -Tom
    { name: 'vendor/jquery.scrollintoview', when: 'always' },

    // PUNKAVE-MAINTAINED, GENERAL PURPOSE JQUERY PLUGINS

    { name: 'vendor/jquery.get-outer-html', when: 'always' },
    { name: 'vendor/jquery.find-by-name', when: 'always' },
    { name: 'vendor/jquery.projector', when: 'always' },
    { name: 'vendor/jquery.bottomless', when: 'always' },
    { name: 'vendor/jquery.selective', when: 'always' },
    { name: 'vendor/jquery.images-ready', when: 'always' },
    { name: 'vendor/jquery.radio', when: 'always' },
    { name: 'vendor/jquery.json-call', when: 'always' },

    // PUNKAVE-MAINTAINED POLYFILLS

    { name: 'vendor/setImmediate', when: 'always' },

    // PUNKAVE-MAINTAINED OOP SYSTEM
    { name: 'vendor/moog', when: 'always' },

    // APOSTROPHE CORE JS

    // Core functionality of the browser-side `apos` object
    { name: 'always', when: 'always' },
    { name: 'user', when: 'user' }
  ];
};

`

boutell commented 4 years ago

We already set it by default on new projects in the CLI, because apostrophe-boilerplate (the repo copied by the CLI) already does set it.

On Wed, Apr 1, 2020 at 11:31 AM Tosh Koevoets notifications@github.com wrote:

Codewise it's not very challenging.

But an update doesn't ensure backwards compatibility, since it forces current users to migrate jQuery to 3. For many it will be without issues, but for others some surprising bugs might happen.

https://jquery.com/upgrade-guide/3.0/

Maybe easier to set it default on in newly created projects in the CLI?

Code for changing the defaults

` self.setDefaultStylesheets = function() { // Default stylesheet requirements self.stylesheets = [ // Must have a jQuery UI theme or acceptable substitute for // autocomplete and datepickers to be usable. -Tom { name: (self.options.jQuery === 1) ? 'vendor/jquery-ui' : 'vendor/jquery-ui-3', when: 'always' },

{ name: 'vendor/pikaday', when: 'always' },
// { name: 'vendor/jquery.Jcrop', when: 'user' }
{ name: 'vendor/cropper', when: 'user' },
{ name: 'vendor/spectrum', when: 'user' }

]; };

self.setDefaultScripts = function() { // Default browser side script requirements // TODO: lots of override options self.scripts = [ // VENDOR DEPENDENCIES

// Call setImmediate safely on browser side; much faster
// than setTimeout(fn, 0)
{ name: 'vendor/setImmediate', when: 'always' },
// For elegant, cross-browser functional-style programming.
// Yes, we are standardized on lodash 3 due to our own development
// cycle.
{ name: 'vendor/lodash', when: 'always' },
// For async code without tears
{ name: 'vendor/async', when: 'always' },
// For manipulating dates and times
{ name: 'vendor/moment', when: 'always' },
// For everything DOM-related
{ name: (self.options.jQuery === 1) ? 'vendor/jquery' : 'vendor/jquery-3', when: 'always' },

// For parsing query parameters browser-side
{ name: 'vendor/jquery-url-parser', when: 'always' },
// For blueimp uploader, drag and drop reordering of anything
// & autocomplete
{ name: (self.options.jQuery === 1) ? 'vendor/jquery-ui' : 'vendor/jquery-ui-3', when: 'always' },
// For the RTE
{ name: 'vendor/jquery-hotkeys', when: 'user' },
// Graceful fallback for older browsers for jquery fileupload
{ name: 'vendor/jquery.iframe-transport', when: 'user' },
// Spiffy multiple file upload
{ name: 'vendor/jquery.fileupload', when: 'user' },
// imaging cropping plugin
// { name: 'vendor/jquery.Jcrop.min', when: 'user' },
{ name: 'vendor/cropper', when: 'user' },
// textchange event, detects actual typing activity, not just focus change
{ name: 'vendor/jquery-textchange', when: 'always' },
// preferred datepicker plugin
{ name: 'vendor/pikaday', when: 'always' },
// Set, get and delete cookies in browser-side JavaScript
{ name: 'vendor/jquery.cookie', when: 'always' },
{ name: 'vendor/jquery.findSafe', when: 'user' },
{ name: 'vendor/jquery.onSafe', when: 'user' },
{ name: 'vendor/jquery.alter-class', when: 'user' },
{ name: 'vendor/sluggo', when: 'user' },

// Spectrum is a colorpicker
{ name: 'vendor/spectrum', when: 'user' },

// Scroll things into view, even if they are in a scrolling
// container which itself needs to be scrolled into view or
// whatever, it's pretty great:
//
// http://erraticdev.blogspot.com/2011/02/jquery-scroll-into-view-plugin-with.html
//
// (Note recent comments, it's actively maintained). -Tom
{ name: 'vendor/jquery.scrollintoview', when: 'always' },

// PUNKAVE-MAINTAINED, GENERAL PURPOSE JQUERY PLUGINS

{ name: 'vendor/jquery.get-outer-html', when: 'always' },
{ name: 'vendor/jquery.find-by-name', when: 'always' },
{ name: 'vendor/jquery.projector', when: 'always' },
{ name: 'vendor/jquery.bottomless', when: 'always' },
{ name: 'vendor/jquery.selective', when: 'always' },
{ name: 'vendor/jquery.images-ready', when: 'always' },
{ name: 'vendor/jquery.radio', when: 'always' },
{ name: 'vendor/jquery.json-call', when: 'always' },

// PUNKAVE-MAINTAINED POLYFILLS

{ name: 'vendor/setImmediate', when: 'always' },

// PUNKAVE-MAINTAINED OOP SYSTEM
{ name: 'vendor/moog', when: 'always' },

// APOSTROPHE CORE JS

// Core functionality of the browser-side `apos` object
{ name: 'always', when: 'always' },
{ name: 'user', when: 'user' }

]; };

`

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2167#issuecomment-607319570, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27K2KLXDYBUUI4QFLZLRKNM4RANCNFSM4LY4IUPQ .

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell commented 4 years ago

I think we should call self.apos.utils.warnDev when we don't see it.

On Wed, Apr 1, 2020 at 11:33 AM Tom Boutell tom@apostrophecms.com wrote:

We already set it by default on new projects in the CLI, because apostrophe-boilerplate (the repo copied by the CLI) already does set it.

On Wed, Apr 1, 2020 at 11:31 AM Tosh Koevoets notifications@github.com wrote:

Codewise it's not very challenging.

But an update doesn't ensure backwards compatibility, since it forces current users to migrate jQuery to 3. For many it will be without issues, but for others some surprising bugs might happen.

https://jquery.com/upgrade-guide/3.0/

Maybe easier to set it default on in newly created projects in the CLI?

Code for changing the defaults

` self.setDefaultStylesheets = function() { // Default stylesheet requirements self.stylesheets = [ // Must have a jQuery UI theme or acceptable substitute for // autocomplete and datepickers to be usable. -Tom { name: (self.options.jQuery === 1) ? 'vendor/jquery-ui' : 'vendor/jquery-ui-3', when: 'always' },

{ name: 'vendor/pikaday', when: 'always' },
// { name: 'vendor/jquery.Jcrop', when: 'user' }
{ name: 'vendor/cropper', when: 'user' },
{ name: 'vendor/spectrum', when: 'user' }

]; };

self.setDefaultScripts = function() { // Default browser side script requirements // TODO: lots of override options self.scripts = [ // VENDOR DEPENDENCIES

// Call setImmediate safely on browser side; much faster
// than setTimeout(fn, 0)
{ name: 'vendor/setImmediate', when: 'always' },
// For elegant, cross-browser functional-style programming.
// Yes, we are standardized on lodash 3 due to our own development
// cycle.
{ name: 'vendor/lodash', when: 'always' },
// For async code without tears
{ name: 'vendor/async', when: 'always' },
// For manipulating dates and times
{ name: 'vendor/moment', when: 'always' },
// For everything DOM-related
{ name: (self.options.jQuery === 1) ? 'vendor/jquery' : 'vendor/jquery-3', when: 'always' },

// For parsing query parameters browser-side
{ name: 'vendor/jquery-url-parser', when: 'always' },
// For blueimp uploader, drag and drop reordering of anything
// & autocomplete
{ name: (self.options.jQuery === 1) ? 'vendor/jquery-ui' : 'vendor/jquery-ui-3', when: 'always' },
// For the RTE
{ name: 'vendor/jquery-hotkeys', when: 'user' },
// Graceful fallback for older browsers for jquery fileupload
{ name: 'vendor/jquery.iframe-transport', when: 'user' },
// Spiffy multiple file upload
{ name: 'vendor/jquery.fileupload', when: 'user' },
// imaging cropping plugin
// { name: 'vendor/jquery.Jcrop.min', when: 'user' },
{ name: 'vendor/cropper', when: 'user' },
// textchange event, detects actual typing activity, not just focus change
{ name: 'vendor/jquery-textchange', when: 'always' },
// preferred datepicker plugin
{ name: 'vendor/pikaday', when: 'always' },
// Set, get and delete cookies in browser-side JavaScript
{ name: 'vendor/jquery.cookie', when: 'always' },
{ name: 'vendor/jquery.findSafe', when: 'user' },
{ name: 'vendor/jquery.onSafe', when: 'user' },
{ name: 'vendor/jquery.alter-class', when: 'user' },
{ name: 'vendor/sluggo', when: 'user' },

// Spectrum is a colorpicker
{ name: 'vendor/spectrum', when: 'user' },

// Scroll things into view, even if they are in a scrolling
// container which itself needs to be scrolled into view or
// whatever, it's pretty great:
//
// http://erraticdev.blogspot.com/2011/02/jquery-scroll-into-view-plugin-with.html
//
// (Note recent comments, it's actively maintained). -Tom
{ name: 'vendor/jquery.scrollintoview', when: 'always' },

// PUNKAVE-MAINTAINED, GENERAL PURPOSE JQUERY PLUGINS

{ name: 'vendor/jquery.get-outer-html', when: 'always' },
{ name: 'vendor/jquery.find-by-name', when: 'always' },
{ name: 'vendor/jquery.projector', when: 'always' },
{ name: 'vendor/jquery.bottomless', when: 'always' },
{ name: 'vendor/jquery.selective', when: 'always' },
{ name: 'vendor/jquery.images-ready', when: 'always' },
{ name: 'vendor/jquery.radio', when: 'always' },
{ name: 'vendor/jquery.json-call', when: 'always' },

// PUNKAVE-MAINTAINED POLYFILLS

{ name: 'vendor/setImmediate', when: 'always' },

// PUNKAVE-MAINTAINED OOP SYSTEM
{ name: 'vendor/moog', when: 'always' },

// APOSTROPHE CORE JS

// Core functionality of the browser-side `apos` object
{ name: 'always', when: 'always' },
{ name: 'user', when: 'user' }

]; };

`

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2167#issuecomment-607319570, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27K2KLXDYBUUI4QFLZLRKNM4RANCNFSM4LY4IUPQ .

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

ToshKoevoets commented 4 years ago

Aha, apologies, wsnt aware the defaults are taken from there, I've probably generated our projects before that happened. Then for now I think warnDev is fine indeed.

ToshKoevoets commented 4 years ago

Btw thanks for the great product, we have been using it to create a open democracy platform for the city of Amsterdam, running around 20 sites on a multisite version with seperate api's and a bunch of our own widgets. One other city in the Netherlands is using it now, 3 more will start using it. Our code needs a lot of improvement but without your CMS it wasn't possible. Looking forward to version 3! When our code matures a bit, hopefully we can also contribute something back.

abea commented 4 years ago

That sounds amazing! We'd love to hear more as you work on it and they go live.

ToshKoevoets commented 4 years ago

Sure, where I can send some urls we worked on? We just launched a a corona platform the city of Amsterdam: https://wijamsterdam.nl/

abea commented 4 years ago

Awesome. The Discord community or abea@apostrophecms.com are great.