CODEWITHSUNDEEP
securitymeteor
w3jimmy
w3jimmy

Reputation: 712

How to prevent malicious Meteor subscriptions using check

I found an important security fault in my meteor app regarding subscriptions (maybe methods are also affected by this).

Even though I use the check package and check() assuring that the correct parameters data types are received inside the publication, I have realised that if a user maliciously subscribes to that subscription with wrong parameter data types it is affecting all other users that are using the same subscription because the meteor server is not running the publication while the malicious user is using incorrect parameters.

How can I prevent this?

Packages used:

aldeed:[email protected]
[email protected]
mdg:validated-method

and npm

import { check, Match } from 'meteor/check';

Server side:

Meteor.publish('postersPub', function postersPub(params) {
    check(params, {
        size: String,
        section: String,
    });
    return Posters.find({
        section: params.section,
        size: params.size === 'large' ? 'large' : 'small',
    }, {
        // fields: { ... }
        // sort: { ... }
    });
});

Client side:

// in the template:
Meteor.subscribe('postersPub', { size: 'large', section: 'movies' });

// Malicious user in the browser console:
Meteor.subscribe('postersPub', { size: undefined, section: '' });

Problem: The malicious user subscription is preventing all other users of getting answer from their postersPub subscriptions.

Extra note: I've also tried wrapping the check block AND the whole publication with a try catch and it doesn't change the effect. The error disappears from the server console, but the other users keep being affected and not getting data from the subscription that the malicious user is affecting.

Upvotes: 2

Views: 99

Answers (1)

Jankapunkt
Jankapunkt

Reputation: 8423

Check method and empty strings

There is one thing to know about check and strings which is, that it accepts empty strings like '' which you basically showed in your malicious example.

Without guarantee to solve your publication issue I can at least suggest you to modify your check code and include a check for non-empty Strings.

A possible approach could be:

import { check, Match } from 'meteor/check';

const nonEmptyString = Match.Where(str => typeof str === 'string' && str.length > 0);

which then can be used in check like so:

check(params, {
    size: nonEmptyString,
    section: nonEmptyString,
});


Even more strict checks

You may be even stricter with accepted parameters and reduce them to a subset of valid entries. For example:

const sizes = ['large', 'small'];
const nonEmptyString = str => typeof str === 'string' && str.length > 0;
const validSize = str => nonEmptyString(str) && sizes.indexOf( str) > -1;

check(params, {
    size: Match.Where(validSize),
    section: Match.Where(nonEmptyString),
});

Note, that this also helps you to avoid query logic based on the parameter. You can change the following code

const posters = Posters.find({
    section: params.section,
    size: params.size === 'large' ? 'large' : 'small',
}, {
    // fields: { ... }
    // sort: { ... }
});

to 

const posters = Posters.find({
    section: params.section,
    size: params.size,
}, {
    // fields: { ... }
    // sort: { ... }
});

because the method does anyway accept only one of large or small as parameters.


Fallback on undefined cursors in publications

Another pattern that can support you preventing publication errors is to call this.ready() if the collection returned no cursor (for whatever reason, better is to write good tests to prevent you from these cases).

const posters = Posters.find({
    section: params.section,
    size: params.size === 'large' ? 'large' : 'small',
}, {
    // fields: { ... }
    // sort: { ... }
});

// if we have a cursor with count
if (posters && posters.count && posters.count() >= 0)
    return posters;

// else signal the subscription
// that we are ready
this.ready();


Combined code example

Applying all of the above mentioned pattern would make your function look like this:

import { check, Match } from 'meteor/check';

const sizes = ['large', 'small'];
const nonEmptyString = str => typeof str === 'string' && str.length > 0;
const validSize = str => nonEmptyString(str) && sizes.indexOf( str) > -1;



Meteor.publish('postersPub', function postersPub(params) {
    check(params, {
        size: Match.Where(validSize),
        section: Match.Where(nonEmptyString),
    });


    const posters = Posters.find({
        section: params.section,
        size: params.size,
    }, {
        // fields: { ... }
        // sort: { ... }
    });

    // if we have a cursor with count
    if (posters && posters.count && posters.count() >= 0)
        return posters;

    // else signal the subscription
    // that we are ready
    this.ready();
});


Summary

I for myself found that with good check matches and this.ready() the problems with publications have been reduced to a minimum in my applications.

Upvotes: 5

Related Questions