diff --git a/lib/handlers/publish.js b/lib/handlers/publish.js index 10ea62fc..f3c8cd87 100644 --- a/lib/handlers/publish.js +++ b/lib/handlers/publish.js @@ -17,6 +17,22 @@ var publishActions = [ enqueuePublish ] function handlePublish (client, packet, done) { + var topic = packet.topic + var err + if (topic.length === 0) { + err = new Error('empty topic not allowed in PUBLISH') + return done(err) + } + for (var i = 0; i < topic.length; i++) { + switch (topic.charCodeAt(i)) { + case 35: + err = new Error('# is not allowed in PUBLISH') + return done(err) + case 43: + err = new Error('+ is not allowed in PUBLISH') + return done(err) + } + } client.broker._series(client, publishActions, packet, done) } diff --git a/lib/handlers/subscribe.js b/lib/handlers/subscribe.js index 1796f3ce..ece4cb0f 100644 --- a/lib/handlers/subscribe.js +++ b/lib/handlers/subscribe.js @@ -4,6 +4,7 @@ var write = require('../write') var fastfall = require('fastfall') var Packet = require('aedes-packet') var through = require('through2') +var validateTopic = require('./validations').validateTopic var topicActions = fastfall([ authorize, storeSubscriptions, @@ -36,6 +37,10 @@ function doSubscribe (sub, done) { function authorize (sub, done) { var client = this.client + var err = validateTopic(sub.topic, 'SUBSCRIBE') + if (err) { + return done(err) + } client.broker.authorizeSubscribe(client, sub, done) } diff --git a/lib/handlers/unsubscribe.js b/lib/handlers/unsubscribe.js index 927eda52..8b4700a8 100644 --- a/lib/handlers/unsubscribe.js +++ b/lib/handlers/unsubscribe.js @@ -1,6 +1,7 @@ 'use strict' var write = require('../write') +var validateTopic = require('./validations').validateTopic function UnsubscribeState (client, packet, finish, granted) { this.client = client @@ -11,9 +12,18 @@ function UnsubscribeState (client, packet, finish, granted) { function handleUnsubscribe (client, packet, done) { var broker = client.broker + var unsubscriptions = packet.unsubscriptions + var err + + for (var i = 0; i < unsubscriptions.length; i++) { + err = validateTopic(unsubscriptions[i], 'UNSUBSCRIBE') + if (err) { + return done(err) + } + } if (packet.messageId) { - broker.persistence.removeSubscriptions(client, packet.unsubscriptions, function (err) { + broker.persistence.removeSubscriptions(client, unsubscriptions, function (err) { if (err) { return done(err) } diff --git a/lib/handlers/validations.js b/lib/handlers/validations.js new file mode 100644 index 00000000..16d9922b --- /dev/null +++ b/lib/handlers/validations.js @@ -0,0 +1,29 @@ +'use strict' + +function validateTopic (topic, message) { + var end = topic.length - 1 + var endMinus = end - 1 + var slashInPreEnd = endMinus > 0 && topic.charCodeAt(endMinus) !== 47 + if (topic.length === 0) { + return new Error('impossible to ' + message + ' to an empty topic') + } + for (var i = 0; i < topic.length; i++) { + switch (topic.charCodeAt(i)) { + case 35: + var notAtTheEnd = i !== end + if (notAtTheEnd || slashInPreEnd) { + return new Error('# is only allowed in ' + message + ' in the last position') + } + break + case 43: + var pastChar = i < end - 1 && topic.charCodeAt(i + 1) !== 47 + var preChar = i > 1 && topic.charCodeAt(i - 1) !== 47 + if (pastChar || preChar) { + return new Error('+ is only allowed in ' + message + ' between /') + } + break + } + } +} + +module.exports.validateTopic = validateTopic diff --git a/test/basic.js b/test/basic.js index f8a6e835..fb0ee60b 100644 --- a/test/basic.js +++ b/test/basic.js @@ -486,3 +486,99 @@ test('avoid wrong deduping of retain messages', function (t) { publisher.inStream.write(expected) }) + +test('publish empty topic', function (t) { + var s = connect(setup()) + + subscribe(t, s, '#', 0, function () { + s.outStream.once('data', function (packet) { + t.fail('no packet') + t.end() + }) + + s.inStream.write({ + cmd: 'publish', + topic: '', + payload: 'world' + }) + }) + + eos(s.conn, function () { + t.equal(s.broker.connectedClients, 0, 'no connected clients') + t.end() + }) +}) + +test('publish invalid topic with #', function (t) { + var s = connect(setup()) + + subscribe(t, s, '#', 0, function () { + s.outStream.once('data', function (packet) { + t.fail('no packet') + t.end() + }) + + s.inStream.write({ + cmd: 'publish', + topic: 'hello/#', + payload: 'world' + }) + }) + + s.broker.on('clientError', function () { + t.end() + }) +}) + +test('publish invalid topic with +', function (t) { + var s = connect(setup()) + + subscribe(t, s, '#', 0, function () { + s.outStream.once('data', function (packet) { + t.fail('no packet') + }) + + s.inStream.write({ + cmd: 'publish', + topic: 'hello/+/eee', + payload: 'world' + }) + }) + + s.broker.on('clientError', function () { + t.end() + }) +}) + +;['base/#/sub', 'base/#sub', 'base/sub#', 'base/xyz+/sub', 'base/+xyz/sub'].forEach(function (topic) { + test('subscribe to invalid topic with "' + topic + '"', function (t) { + var s = connect(setup()) + + s.broker.on('clientError', function () { + t.end() + }) + + s.inStream.write({ + cmd: 'subscribe', + messageId: 24, + subscriptions: [{ + topic: topic, + qos: 0 + }] + }) + }) + + test('unsubscribe to invalid topic with "' + topic + '"', function (t) { + var s = connect(setup()) + + s.broker.on('clientError', function () { + t.end() + }) + + s.inStream.write({ + cmd: 'unsubscribe', + messageId: 24, + unsubscriptions: [topic] + }) + }) +})