From aa80521cb25b7df076826ca5f8b3d321a35944dc Mon Sep 17 00:00:00 2001 From: Dan Dascalescu Date: Sun, 24 May 2015 20:09:59 -0700 Subject: [PATCH] Fix security issue allowing modification of arbitrary numeric fields --- meteor/README.md | 12 ++ meteor/example/.meteor/.finished-upgraders | 8 ++ meteor/example/.meteor/.id | 7 ++ meteor/example/.meteor/platforms | 2 + meteor/example/.meteor/release | 2 +- meteor/example/.meteor/versions | 109 +++++++++--------- meteor/example/README.md | 2 +- meteor/example/package.json | 1 - meteor/example/server/sortable-collections.js | 3 + meteor/{methods.js => methods-client.js} | 10 +- meteor/methods-server.js | 31 +++++ meteor/package.js | 9 +- meteor/reactivize.js | 2 +- 13 files changed, 127 insertions(+), 71 deletions(-) create mode 100644 meteor/example/.meteor/.finished-upgraders create mode 100644 meteor/example/.meteor/.id create mode 100644 meteor/example/.meteor/platforms delete mode 120000 meteor/example/package.json create mode 100644 meteor/example/server/sortable-collections.js rename meteor/{methods.js => methods-client.js} (64%) create mode 100644 meteor/methods-server.js diff --git a/meteor/README.md b/meteor/README.md index 0faf2b2..c340358 100644 --- a/meteor/README.md +++ b/meteor/README.md @@ -25,10 +25,18 @@ Simplest invocation - order will be lost when the page is refreshed: Persist the sort order in the 'order' field of each document in the collection: +*Client:* + ```handlebars {{#sortable items= sortField="order"}} ``` +*Server:* + +```js +Sortable.collections = ; // the name, not the variable +``` + Along with `items`, `sortField` is the only Meteor-specific option. If it's missing, the package will assume there is a field called "order" in the collection, holding unique `Number`s such that every `order` differs from that before and after it by at least 1. Basically, keep to 0, 1, 2, ... . @@ -36,6 +44,10 @@ Try not to depend on a particular format for this field; it *is* though guarante produce lexicographical order, and that the order will be maintained after an arbitrary number of reorderings, unlike with [naive solutions](http://programmers.stackexchange.com/questions/266451/maintain-ordered-collection-by-updating-as-few-order-fields-as-possible). +Remember to declare on the server which collections you want to be reorderable from the client. +Otherwise, the library will error because the client would be able to modify numerical fields in +any collection, which represents a security risk. + ## Passing options to the Sortable library diff --git a/meteor/example/.meteor/.finished-upgraders b/meteor/example/.meteor/.finished-upgraders new file mode 100644 index 0000000..8a76103 --- /dev/null +++ b/meteor/example/.meteor/.finished-upgraders @@ -0,0 +1,8 @@ +# This file contains information which helps Meteor properly upgrade your +# app when you run 'meteor update'. You should check it into version control +# with your project. + +notices-for-0.9.0 +notices-for-0.9.1 +0.9.4-platform-file +notices-for-facebook-graph-api-2 diff --git a/meteor/example/.meteor/.id b/meteor/example/.meteor/.id new file mode 100644 index 0000000..b39baa1 --- /dev/null +++ b/meteor/example/.meteor/.id @@ -0,0 +1,7 @@ +# This file contains a token that is unique to your project. +# Check it into your repository along with the rest of this directory. +# It can be used for purposes such as: +# - ensuring you don't accidentally deploy one app on top of another +# - providing package authors with aggregated statistics + +ir0jg2douy3yo5mehw diff --git a/meteor/example/.meteor/platforms b/meteor/example/.meteor/platforms new file mode 100644 index 0000000..8a3a35f --- /dev/null +++ b/meteor/example/.meteor/platforms @@ -0,0 +1,2 @@ +browser +server diff --git a/meteor/example/.meteor/release b/meteor/example/.meteor/release index f1b6255..dab6b55 100644 --- a/meteor/example/.meteor/release +++ b/meteor/example/.meteor/release @@ -1 +1 @@ -METEOR@1.0.1 +METEOR@1.1.0.2 diff --git a/meteor/example/.meteor/versions b/meteor/example/.meteor/versions index ce300af..abcbcf0 100644 --- a/meteor/example/.meteor/versions +++ b/meteor/example/.meteor/versions @@ -1,56 +1,53 @@ -application-configuration@1.0.3 -autopublish@1.0.1 -autoupdate@1.1.3 -base64@1.0.1 -binary-heap@1.0.1 -blaze-tools@1.0.1 -blaze@2.0.3 -boilerplate-generator@1.0.1 -callback-hook@1.0.1 -check@1.0.2 -ctl-helper@1.0.4 -ctl@1.0.2 -dburles:mongo-collection-instances@0.2.5 -ddp@1.0.12 -deps@1.0.5 -ejson@1.0.4 -fastclick@1.0.1 -fezvrasta:bootstrap-material-design@0.2.1 -follower-livedata@1.0.2 -geojson-utils@1.0.1 -html-tools@1.0.2 -htmljs@1.0.2 -http@1.0.8 -id-map@1.0.1 -insecure@1.0.1 -jquery@1.0.1 -json@1.0.1 -launch-screen@1.0.0 -livedata@1.0.11 -logging@1.0.5 -meteor-platform@1.2.0 -meteor@1.1.3 -minifiers@1.1.2 -minimongo@1.0.5 -mobile-status-bar@1.0.1 -mongo@1.0.9 -observe-sequence@1.0.3 -ordered-dict@1.0.1 -random@1.0.1 -reactive-dict@1.0.4 -reactive-var@1.0.3 -reload@1.1.1 -retry@1.0.1 -routepolicy@1.0.2 -rubaxa:sortable@1.0.0 -session@1.0.4 -spacebars-compiler@1.0.3 -spacebars@1.0.3 -templating@1.0.9 -tracker@1.0.3 -twbs:bootstrap@3.3.1 -ui@1.0.4 -underscore@1.0.1 -url@1.0.2 -webapp-hashing@1.0.1 -webapp@1.1.4 +autopublish@1.0.3 +autoupdate@1.2.1 +base64@1.0.3 +binary-heap@1.0.3 +blaze@2.1.2 +blaze-tools@1.0.3 +boilerplate-generator@1.0.3 +callback-hook@1.0.3 +check@1.0.5 +dburles:mongo-collection-instances@0.3.3 +ddp@1.1.0 +deps@1.0.7 +ejson@1.0.6 +fastclick@1.0.3 +fezvrasta:bootstrap-material-design@0.3.0 +geojson-utils@1.0.3 +html-tools@1.0.4 +htmljs@1.0.4 +http@1.1.0 +id-map@1.0.3 +insecure@1.0.3 +jquery@1.11.3_2 +json@1.0.3 +lai:collection-extensions@0.1.3 +launch-screen@1.0.2 +livedata@1.0.13 +logging@1.0.7 +meteor@1.1.6 +meteor-platform@1.2.2 +minifiers@1.1.5 +minimongo@1.0.8 +mobile-status-bar@1.0.3 +mongo@1.1.0 +observe-sequence@1.0.6 +ordered-dict@1.0.3 +random@1.0.3 +reactive-dict@1.1.0 +reactive-var@1.0.5 +reload@1.1.3 +retry@1.0.3 +routepolicy@1.0.5 +rubaxa:sortable@1.2.0 +session@1.1.0 +spacebars@1.0.6 +spacebars-compiler@1.0.6 +templating@1.1.1 +tracker@1.0.7 +twbs:bootstrap@3.3.4 +ui@1.0.6 +underscore@1.0.3 +url@1.0.4 +webapp@1.2.0 +webapp-hashing@1.0.3 diff --git a/meteor/example/README.md b/meteor/example/README.md index 5e48aca..51d1547 100644 --- a/meteor/example/README.md +++ b/meteor/example/README.md @@ -35,7 +35,7 @@ run script: ### Differential Differential wrote [a blog post on reorderable lists with -Meteor](differential.com/blog/sortable-lists-in-meteor-using-jquery-ui) and +Meteor](http://differential.com/blog/sortable-lists-in-meteor-using-jquery-ui) and [jQuery UI Sortable](http://jqueryui.com/sortable/). It served as inspiration for integrating [rubaxa:sortable](rubaxa.github.io/Sortable/), which uses the HTML5 native drag&drop API (not without [its diff --git a/meteor/example/package.json b/meteor/example/package.json deleted file mode 120000 index 138a42c..0000000 --- a/meteor/example/package.json +++ /dev/null @@ -1 +0,0 @@ -../../package.json \ No newline at end of file diff --git a/meteor/example/server/sortable-collections.js b/meteor/example/server/sortable-collections.js new file mode 100644 index 0000000..76069a5 --- /dev/null +++ b/meteor/example/server/sortable-collections.js @@ -0,0 +1,3 @@ +'use strict'; + +Sortable.collections = ['attributes']; diff --git a/meteor/methods.js b/meteor/methods-client.js similarity index 64% rename from meteor/methods.js rename to meteor/methods-client.js index fe8d834..52f4ebe 100644 --- a/meteor/methods.js +++ b/meteor/methods-client.js @@ -2,19 +2,15 @@ Meteor.methods({ /** - * Update the orderField of documents with given ids in a collection, incrementing it by incDec + * Update the sortField of documents with given ids in a collection, incrementing it by incDec * @param {String} collectionName - name of the collection to update * @param {String[]} ids - array of document ids * @param {String} orderField - the name of the order field, usually "order" * @param {Number} incDec - pass 1 or -1 */ - 'rubaxa:sortable/collection-update': function (collectionName, ids, orderField, incDec) { - check(collectionName, String); - check(ids, [String]); - check(orderField, String); - check(incDec, Number); + 'rubaxa:sortable/collection-update': function (collectionName, ids, sortField, incDec) { var selector = {_id: {$in: ids}}, modifier = {$inc: {}}; - modifier.$inc[orderField] = incDec; + modifier.$inc[sortField] = incDec; Mongo.Collection.get(collectionName).update(selector, modifier, {multi: true}); } }); diff --git a/meteor/methods-server.js b/meteor/methods-server.js new file mode 100644 index 0000000..9598bfc --- /dev/null +++ b/meteor/methods-server.js @@ -0,0 +1,31 @@ +'use strict'; + +Sortable = {}; +Sortable.collections = []; // array of collection names that the client is allowed to reorder + +Meteor.methods({ + /** + * Update the sortField of documents with given ids in a collection, incrementing it by incDec + * @param {String} collectionName - name of the collection to update + * @param {String[]} ids - array of document ids + * @param {String} orderField - the name of the order field, usually "order" + * @param {Number} incDec - pass 1 or -1 + */ + 'rubaxa:sortable/collection-update': function (collectionName, ids, sortField, incDec) { + check(collectionName, String); + // don't allow the client to modify just any collection + if (!Sortable || !Array.isArray(Sortable.collections)) { + throw new Meteor.Error(500, 'Please define Sortable.collections'); + } + if (Sortable.collections.indexOf(collectionName) === -1) { + throw new Meteor.Error(403, 'Collection <' + collectionName + '> is not Sortable. Please add it to Sortable.collections in server code.'); + } + + check(ids, [String]); + check(sortField, String); + check(incDec, Number); + var selector = {_id: {$in: ids}}, modifier = {$inc: {}}; + modifier.$inc[sortField] = incDec; + Mongo.Collection.get(collectionName).update(selector, modifier, {multi: true}); + } +}); diff --git a/meteor/package.js b/meteor/package.js index 148561a..51bf04b 100644 --- a/meteor/package.js +++ b/meteor/package.js @@ -10,20 +10,21 @@ Package.describe({ summary: 'Sortable: reactive minimalist reorderable drag-and-drop lists on modern browsers and touch devices', version: packageJson.version, git: 'https://github.com/RubaXa/Sortable.git', - readme: 'https://github.com/RubaXa/Sortable/blob/master/meteor/README.md' + documentation: 'meteor/README.md' }); Package.onUse(function (api) { api.versionsFrom(['METEOR@0.9.0', 'METEOR@1.0']); api.use('templating', 'client'); - api.use('dburles:mongo-collection-instances@0.2.6'); // to watch collections getting created - api.export('Sortable'); + api.use('dburles:mongo-collection-instances@0.3.3'); // to watch collections getting created + api.export('Sortable'); // exported on the server too, as a global to hold the array of sortable collections (for security) api.addFiles([ 'Sortable.js', 'meteor/template.html', // the HTML comes first, so reactivize.js can refer to the template in it 'meteor/reactivize.js' ], 'client'); - api.addFiles('meteor/methods.js'); // add to both client and server + api.addFiles('meteor/methods-client.js', 'client'); + api.addFiles('meteor/methods-server.js', 'server'); }); Package.onTest(function (api) { diff --git a/meteor/reactivize.js b/meteor/reactivize.js index 34ff501..a068a5e 100644 --- a/meteor/reactivize.js +++ b/meteor/reactivize.js @@ -1,6 +1,6 @@ /* Make a Sortable reactive by binding it to a Mongo.Collection. -Calls `rubaxa:sortable/collection-update` on the server to update the sortField or affected records. +Calls `rubaxa:sortable/collection-update` on the server to update the sortField of affected records. TODO: * supply consecutive values if the `order` field doesn't have any