From 5ca8a8c9d777b64b6d38a612ab26222aa46714d5 Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Tue, 30 Jul 2019 16:25:45 -0300 Subject: [PATCH 1/2] Add validation on urls inside message object --- app/lib/server/functions/sendMessage.js | 19 ++- tests/end-to-end/api/05-chat.js | 199 ++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 7 deletions(-) diff --git a/app/lib/server/functions/sendMessage.js b/app/lib/server/functions/sendMessage.js index d12668baaacf..758bbd684001 100644 --- a/app/lib/server/functions/sendMessage.js +++ b/app/lib/server/functions/sendMessage.js @@ -6,6 +6,7 @@ import { callbacks } from '../../../callbacks'; import { Messages } from '../../../models'; import { Apps } from '../../../apps/server'; import { Markdown } from '../../../markdown/server'; +import { isURL } from '../../../utils/lib/isURL'; /** * IMPORTANT @@ -19,6 +20,10 @@ import { Markdown } from '../../../markdown/server'; const ValidHref = Match.Where((value) => { check(value, String); + if (!isURL(value)) { + throw new Error('Invalid href value provided'); + } + if (/^javascript:/i.test(value)) { throw new Error('Invalid href value provided'); } @@ -58,7 +63,7 @@ const validateAttachmentsActions = (attachmentActions) => { type: String, text: String, url: ValidHref, - image_url: String, + image_url: ValidHref, is_webview: Boolean, webview_height_ratio: String, msg: String, @@ -71,26 +76,26 @@ const validateAttachment = (attachment) => { color: String, text: String, ts: Match.OneOf(String, Match.Integer), - thumb_url: String, + thumb_url: ValidHref, button_alignment: String, actions: [Match.Any], message_link: ValidHref, collapsed: Boolean, author_name: String, author_link: ValidHref, - author_icon: String, + author_icon: ValidHref, title: String, title_link: ValidHref, title_link_download: Boolean, image_dimensions: Object, - image_url: String, + image_url: ValidHref, image_preview: String, image_type: String, image_size: Number, - audio_url: String, + audio_url: ValidHref, audio_type: String, audio_size: Number, - video_url: String, + video_url: ValidHref, video_type: String, video_size: Number, fields: [Match.Any], @@ -114,7 +119,7 @@ const validateMessage = (message) => { text: String, alias: String, emoji: String, - avatar: String, + avatar: ValidHref, attachments: [Match.Any], })); diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index 04e8f9ea7c97..49f143c57e80 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -183,6 +183,205 @@ describe('[Chat]', function() { }) .end(done) ); + + it('message.avatar', (done) => + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'javascript:alert("xss")', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + title: 'Attachment Example', + title_link: 'https://youtube.com', + actions: [ + { + type: 'button', + text: 'Text', + url: 'https://youtube.com', + }, + ], + }], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done) + ); + + it('attachment.action.image_url', (done) => + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + title: 'Attachment Example', + title_link: 'https://youtube.com', + actions: [ + { + type: 'button', + text: 'Text', + url: 'http://res.guggy.com/logo_128.png', + image_url: 'javascript:alert("xss")', + }, + ], + }], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done) + ); + + it('attachment.thumb_url', (done) => + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + title: 'Attachment Example', + thumb_url: 'javascript:alert("xss")', + title_link: 'http://res.guggy.com/logo_128.png', + }], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done) + ); + + it('attachment.author_icon', (done) => + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + title: 'Attachment Example', + author_icon: 'javascript:alert("xss")', + title_link: 'http://res.guggy.com/logo_128.png', + }], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done) + ); + it('attachment.image_url', (done) => + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + title: 'Attachment Example', + image_url: 'javascript:alert("xss")', + title_link: 'http://res.guggy.com/logo_128.png', + }], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done) + ); + it('attachment.audio_url', (done) => + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + title: 'Attachment Example', + audio_url: 'javascript:alert("xss")', + title_link: 'http://res.guggy.com/logo_128.png', + }], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done) + ); + it('attachment.video_url', (done) => + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + title: 'Attachment Example', + video_url: 'javascript:alert("xss")', + title_link: 'http://res.guggy.com/logo_128.png', + }], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done) + ); }); it('should throw an error when the properties (attachments.fields.title, attachments.fields.value) are with the wrong type', (done) => { From c2b976eb8529dac2058562255d178c79c6e81c32 Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Tue, 30 Jul 2019 19:59:05 -0300 Subject: [PATCH 2/2] rename function --- app/lib/server/functions/sendMessage.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/app/lib/server/functions/sendMessage.js b/app/lib/server/functions/sendMessage.js index 758bbd684001..52e6187de320 100644 --- a/app/lib/server/functions/sendMessage.js +++ b/app/lib/server/functions/sendMessage.js @@ -17,7 +17,7 @@ import { isURL } from '../../../utils/lib/isURL'; * is going to be rendered in the href attribute of a * link. */ -const ValidHref = Match.Where((value) => { +const ValidLinkParam = Match.Where((value) => { check(value, String); if (!isURL(value)) { @@ -62,8 +62,8 @@ const validateAttachmentsActions = (attachmentActions) => { check(attachmentActions, objectMaybeIncluding({ type: String, text: String, - url: ValidHref, - image_url: ValidHref, + url: ValidLinkParam, + image_url: ValidLinkParam, is_webview: Boolean, webview_height_ratio: String, msg: String, @@ -76,26 +76,26 @@ const validateAttachment = (attachment) => { color: String, text: String, ts: Match.OneOf(String, Match.Integer), - thumb_url: ValidHref, + thumb_url: ValidLinkParam, button_alignment: String, actions: [Match.Any], - message_link: ValidHref, + message_link: ValidLinkParam, collapsed: Boolean, author_name: String, - author_link: ValidHref, - author_icon: ValidHref, + author_link: ValidLinkParam, + author_icon: ValidLinkParam, title: String, - title_link: ValidHref, + title_link: ValidLinkParam, title_link_download: Boolean, image_dimensions: Object, - image_url: ValidHref, + image_url: ValidLinkParam, image_preview: String, image_type: String, image_size: Number, - audio_url: ValidHref, + audio_url: ValidLinkParam, audio_type: String, audio_size: Number, - video_url: ValidHref, + video_url: ValidLinkParam, video_type: String, video_size: Number, fields: [Match.Any], @@ -119,7 +119,7 @@ const validateMessage = (message) => { text: String, alias: String, emoji: String, - avatar: ValidHref, + avatar: ValidLinkParam, attachments: [Match.Any], }));