From 9dc82ad444c14b065b29393ac9d056805fc7ac27 Mon Sep 17 00:00:00 2001 From: Calvin Montgomery Date: Mon, 20 Mar 2017 21:37:32 -0700 Subject: [PATCH] Enforce stricter validation on polls --- NEWS.md | 6 ++ config.template.yaml | 3 + package.json | 4 +- src/channel/poll.js | 62 ++++++++++++++++-- src/config.js | 5 +- src/errors.js | 1 + src/io/ioserver.js | 4 +- test/channel/poll.js | 145 +++++++++++++++++++++++++++++++++++++++++++ www/js/util.js | 12 +++- 9 files changed, 231 insertions(+), 11 deletions(-) create mode 100644 test/channel/poll.js diff --git a/NEWS.md b/NEWS.md index 0799ef53..e14fd0db 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,9 @@ +2017-03-20 +========== + +Polls are now more strictly validated, including the number of options. The +default limit is 50 options, which you can configure via `poll.max-options`. + 2017-03-11 ========== diff --git a/config.template.yaml b/config.template.yaml index daac867b..28fdd7c4 100644 --- a/config.template.yaml +++ b/config.template.yaml @@ -250,3 +250,6 @@ service-socket: # Twitch Client ID for the data API (used for VOD lookups) # https://github.com/justintv/Twitch-API/blob/master/authentication.md#developer-setup twitch-client-id: null + +poll: + max-options: 50 diff --git a/package.json b/package.json index 10eb1a01..2f4b4e0a 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "author": "Calvin Montgomery", "name": "CyTube", "description": "Online media synchronizer and chat", - "version": "3.34.3", + "version": "3.34.4", "repository": { "url": "http://github.com/calzoneman/sync" }, @@ -54,7 +54,7 @@ "postinstall": "./postinstall.sh", "server-dev": "babel -D --watch --source-maps --loose es6.destructuring,es6.forOf --out-dir lib/ src/", "generate-userscript": "$npm_node_execpath gdrive-userscript/generate-userscript $@ > www/js/cytube-google-drive.user.js", - "test": "mocha", + "test": "mocha --recursive test", "integration-test": "mocha --recursive integration_test" }, "devDependencies": { diff --git a/src/channel/poll.js b/src/channel/poll.js index da6fa9c8..3a66ca33 100644 --- a/src/channel/poll.js +++ b/src/channel/poll.js @@ -1,5 +1,7 @@ var ChannelModule = require("./module"); var Poll = require("../poll").Poll; +import { ValidationError } from '../errors'; +import Config from '../config'; const TYPE_NEW_POLL = { title: "string", @@ -130,16 +132,55 @@ PollModule.prototype.broadcastPoll = function (isNewPoll) { this.channel.broadcastToRoom(event, obscured, this.roomNoViewHidden); }; -PollModule.prototype.handleNewPoll = function (user, data) { +PollModule.prototype.validatePollInput = function validatePollInput(title, options) { + if (typeof title !== 'string') { + throw new ValidationError('Poll title must be a string.'); + } + if (title.length > 255) { + throw new ValidationError('Poll title must be no more than 255 characters long.'); + } + if (!Array.isArray(options)) { + throw new ValidationError('Poll options must be an array.'); + } + if (options.length > Config.get('poll.max-options')) { + throw new ValidationError(`Polls are limited to a maximum of ${Config.get('poll.max-options')} options.`); + } + for (let i = 0; i < options.length; i++) { + if (typeof options[i] !== 'string') { + throw new ValidationError('Poll options must be strings.'); + } + if (options[i].length === 0 || options[i].length > 255) { + throw new ValidationError('Poll options must be 1-255 characters long.'); + } + } +}; + +PollModule.prototype.handleNewPoll = function (user, data, ack) { if (!this.channel.modules.permissions.canControlPoll(user)) { return; } - var title = data.title.substring(0, 255); - var opts = data.opts.map(function (x) { return (""+x).substring(0, 255); }); - var obscured = data.obscured; + if (typeof data !== 'object' || data === null) { + ack({ + error: { + message: 'Invalid data received for poll creation.' + } + }); + return; + } - var poll = new Poll(user.getName(), title, opts, obscured); + try { + this.validatePollInput(data.title, data.opts); + } catch (error) { + ack({ + error: { + message: error.message + } + }); + return; + } + + var poll = new Poll(user.getName(), data.title, data.opts, data.obscured); var self = this; if (data.hasOwnProperty("timeout") && !isNaN(data.timeout) && data.timeout > 0) { poll.timer = setTimeout(function () { @@ -155,6 +196,7 @@ PollModule.prototype.handleNewPoll = function (user, data) { this.poll = poll; this.broadcastPoll(true); this.channel.logger.log("[poll] " + user.getName() + " opened poll: '" + poll.title + "'"); + ack({}); }; PollModule.prototype.handleVote = function (user, data) { @@ -198,6 +240,16 @@ PollModule.prototype.handlePollCmd = function (obscured, user, msg, meta) { var args = msg.split(","); var title = args.shift(); + + try { + this.validatePollInput(title, args); + } catch (error) { + user.socket.emit('errorMsg', { + msg: 'Error creating poll: ' + error.message + }); + return; + } + var poll = new Poll(user.getName(), title, args, obscured); this.poll = poll; this.broadcastPoll(true); diff --git a/src/config.js b/src/config.js index 7fa285a3..dd5e2216 100644 --- a/src/config.js +++ b/src/config.js @@ -119,7 +119,10 @@ var defaults = { "google-drive": { "html5-hack-enabled": false }, - "twitch-client-id": null + "twitch-client-id": null, + poll: { + "max-options": 50 + } }; /** diff --git a/src/errors.js b/src/errors.js index f5b45089..bd33cb82 100644 --- a/src/errors.js +++ b/src/errors.js @@ -7,3 +7,4 @@ export const CSRFError = createError('CSRFError'); export const HTTPError = createError('HTTPError', { status: HTTPStatus.INTERNAL_SERVER_ERROR }); +export const ValidationError = createError('ValidationError'); \ No newline at end of file diff --git a/src/io/ioserver.js b/src/io/ioserver.js index 9b4132c8..da83ee09 100644 --- a/src/io/ioserver.js +++ b/src/io/ioserver.js @@ -133,14 +133,14 @@ function ipLimitReached(sock) { function addTypecheckedFunctions(sock) { sock.typecheckedOn = function (msg, template, cb) { - sock.on(msg, function (data) { + sock.on(msg, function (data, ack) { typecheck(data, template, function (err, data) { if (err) { sock.emit("errorMsg", { msg: "Unexpected error for message " + msg + ": " + err.message }); } else { - cb(data); + cb(data, ack); } }); }); diff --git a/test/channel/poll.js b/test/channel/poll.js new file mode 100644 index 00000000..4b54b83a --- /dev/null +++ b/test/channel/poll.js @@ -0,0 +1,145 @@ +const PollModule = require('../../lib/channel/poll'); +const assert = require('assert'); +const Config = require('../../lib/config'); + +describe('PollModule', () => { + describe('#validatePollInput', () => { + let pollModule = new PollModule({ uniqueName: 'testChannel', modules: {} }); + + it('accepts valid input', () => { + let title = ''; + for (let i = 0; i < 20; i++) { + title += 'x'; + } + + pollModule.validatePollInput(title, ['ab', 'cd']); + }); + + it('rejects non-string titles', () => { + assert.throws(() => { + pollModule.validatePollInput(null, []); + }, /title/); + }); + + it('rejects invalidly long titles', () => { + let title = ''; + for (let i = 0; i < 256; i++) { + title += 'x'; + } + + assert.throws(() => { + pollModule.validatePollInput(title, []); + }, /title/); + }); + + it('rejects non-array option parameter', () => { + assert.throws(() => { + pollModule.validatePollInput('poll', 1234); + }, /options/); + }); + + it('rejects too many options', () => { + const limit = Config.get('poll.max-options'); + Config.set('poll.max-options', 2); + try { + assert.throws(() => { + pollModule.validatePollInput('poll', ['1', '2', '3', '4']); + }, /maximum of 2 options/); + } finally { + Config.set('poll.max-options', limit); + } + }); + + it('rejects non-string options', () => { + assert.throws(() => { + pollModule.validatePollInput('poll', [null]); + }, /options must be strings/); + }); + + it('rejects invalidly long options', () => { + let option = ''; + for (let i = 0; i < 256; i++) { + option += 'x'; + } + + assert.throws(() => { + pollModule.validatePollInput('poll', [option]); + }, /options must be 1-255 characters/); + }); + }); + + describe('#handleNewPoll', () => { + let fakeChannel = { + uniqueName: 'testChannel', + logger: { + log() { + + } + }, + broadcastToRoom() { + }, + broadcastAll() { + }, + modules: { + permissions: { + canControlPoll() { + return true; + } + } + } + }; + let fakeUser = { + getName() { + return 'testUser'; + } + }; + let pollModule = new PollModule(fakeChannel); + + it('creates a valid poll', () => { + let sentNewPoll = false; + let sentClosePoll = false; + fakeChannel.broadcastToRoom = (event, data, room) => { + if (room === 'testChannel:viewHidden' && event === 'newPoll') { + sentNewPoll = true; + } + }; + fakeChannel.broadcastAll = (event) => { + if (event === 'closePoll') { + sentClosePoll = true; + } + }; + pollModule.handleNewPoll(fakeUser, { + title: 'test poll', + opts: [ + 'option 1', + 'option 2' + ], + obscured: false + }, (ackResult) => { + assert(!ackResult.error, `Unexpected error: ${ackResult.error}`); + }); + assert(sentClosePoll, 'Expected broadcast of closePoll event'); + assert(sentNewPoll, 'Expected broadcast of newPoll event'); + }); + + it('rejects an invalid poll', () => { + fakeChannel.broadcastToRoom = (event, data, room) => { + assert(false, 'Expected no events to be sent'); + }; + fakeChannel.broadcastAll = (event) => { + assert(false, 'Expected no events to be sent'); + }; + const options = []; + for (let i = 0; i < 200; i++) { + options.push('option ' + i); + } + pollModule.handleNewPoll(fakeUser, { + title: 'test poll', + opts: options, + obscured: false + }, (ackResult) => { + assert.equal(ackResult.error.message, 'Polls are limited to a maximum of 50 options.'); + }); + }); + }) +}); \ No newline at end of file diff --git a/www/js/util.js b/www/js/util.js index 751efdc1..78b85b8d 100644 --- a/www/js/util.js +++ b/www/js/util.js @@ -789,6 +789,7 @@ function showPollMenu() { $("").text("Title").appendTo(menu); var title = $("").addClass("form-control") + .attr("maxlength", "255") .attr("type", "text") .appendTo(menu); @@ -820,6 +821,7 @@ function showPollMenu() { function addOption() { $("").addClass("form-control") .attr("type", "text") + .attr("maxlength", "255") .addClass("poll-menu-option") .insertBefore(addbtn); } @@ -859,8 +861,16 @@ function showPollMenu() { opts: opts, obscured: hidden.prop("checked"), timeout: t + }, function ack(result) { + if (result.error) { + modalAlert({ + title: 'Error creating poll', + textContent: result.error.message + }); + } else { + menu.remove(); + } }); - menu.remove(); }); }