From b7dec00fe719ec34495fd206aa1189e2f5acf195 Mon Sep 17 00:00:00 2001 From: Khoi Doan Date: Thu, 8 Nov 2018 12:09:25 -0500 Subject: [PATCH] Add message whitelist functionality to inabox messaging host (#18698) * Add whitelist functionality to inabox messaging host * Refactor getFrameElement_ to return the entire AdFrameDef * Fix lint errors * Change how whitelist is matched using regex * Fix missing semicolon * Escaping the type before putting it in the regex. * Minor fix - line is short enough * Switch back to using split. --- ads/inabox/inabox-messaging-host.js | 20 +++-- .../inabox/test-inabox-messaging-host.js | 73 ++++++++++++++++--- 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/ads/inabox/inabox-messaging-host.js b/ads/inabox/inabox-messaging-host.js index e4891f486587..76635c2a4b20 100644 --- a/ads/inabox/inabox-messaging-host.js +++ b/ads/inabox/inabox-messaging-host.js @@ -122,15 +122,23 @@ export class InaboxMessagingHost { return false; } - const iframe = + const adFrame = this.getFrameElement_(message.source, request['sentinel']); - if (!iframe) { + if (!adFrame) { dev().info(TAG, 'Ignored message from untrusted iframe:', message); return false; } + const allowedTypes = adFrame.iframe.dataset['ampAllowed']; + // having no whitelist is legacy behavior so assume all types are allowed + if (allowedTypes && + !allowedTypes.split(/\s*,\s*/).includes(request['type'])) { + dev().info(TAG, 'Ignored non-whitelisted message type:', message); + return false; + } + if (!this.msgObservable_.fire(request['type'], this, - [iframe, request, message.source, message.origin])) { + [adFrame.measurableFrame, request, message.source, message.origin])) { dev().warn(TAG, 'Unprocessed AMP message:', message); return false; } @@ -246,12 +254,12 @@ export class InaboxMessagingHost { * * @param {?Window} source * @param {string} sentinel - * @return {?HTMLIFrameElement} + * @return {?AdFrameDef} * @private */ getFrameElement_(source, sentinel) { if (this.iframeMap_[sentinel]) { - return this.iframeMap_[sentinel].measurableFrame; + return this.iframeMap_[sentinel]; } const measurableFrame = this.getMeasureableFrame(source); if (!measurableFrame) { @@ -264,7 +272,7 @@ export class InaboxMessagingHost { j < 10; j++, tempWin = tempWin.parent) { if (iframe.contentWindow == tempWin) { this.iframeMap_[sentinel] = {iframe, measurableFrame}; - return measurableFrame; + return this.iframeMap_[sentinel]; } if (tempWin == window.top) { break; diff --git a/test/functional/inabox/test-inabox-messaging-host.js b/test/functional/inabox/test-inabox-messaging-host.js index f77fd30c4cc6..2c6abdc9e10c 100644 --- a/test/functional/inabox/test-inabox-messaging-host.js +++ b/test/functional/inabox/test-inabox-messaging-host.js @@ -24,20 +24,30 @@ describes.realWin('inabox-host:messaging', {}, env => { let host; let iframe1; let iframe2; + let iframe3; let iframeUntrusted; beforeEach(() => { win = env.win; iframe1 = win.document.createElement('iframe'); iframe2 = win.document.createElement('iframe'); + iframe3 = win.document.createElement('iframe'); iframeUntrusted = win.document.createElement('iframe'); win.document.body.appendChild(iframe1); win.document.body.appendChild(iframe2); + win.document.body.appendChild(iframe3); win.document.body.appendChild(iframeUntrusted); iframe1.contentWindow.postMessage = () => {}; iframe2.contentWindow.postMessage = () => {}; + iframe3.contentWindow.postMessage = () => {}; iframeUntrusted.contentWindow.postMessage = () => {}; - host = new InaboxMessagingHost(win, [iframe1, iframe2]); + iframe1.dataset.ampAllowed = + 'send-positions,full-overlay-frame,cancel-full-overlay-frame'; + iframe2.dataset.ampAllowed = + 'send-positions'; + iframeUntrusted.dataset.ampAllowed = + 'send-positions,full-overlay-frame,cancel-full-overlay-frame'; + host = new InaboxMessagingHost(win, [iframe1, iframe2, iframe3]); }); describe('processMessage', () => { @@ -53,6 +63,17 @@ describes.realWin('inabox-host:messaging', {}, env => { })).to.be.true; }); + it('should process valid message 2', () => { + expect(host.processMessage({ + source: iframe1.contentWindow, + origin: 'www.example.com', + data: 'amp-' + JSON.stringify({ + sentinel: '0-123', + type: 'full-overlay-frame', + }), + })).to.be.true; + }); + it('should ignore non-string message', () => { expect(host.processMessage({ source: iframe1.contentWindow, @@ -103,6 +124,39 @@ describes.realWin('inabox-host:messaging', {}, env => { }), }); }); + + it('should process messages with allowed actions', () => { + expect(host.processMessage({ + source: iframe2.contentWindow, + origin: 'www.example.com', + data: 'amp-' + JSON.stringify({ + sentinel: '0-124', + type: 'send-positions', + }), + })).to.be.true; + }); + + it('should ignore messages with disallowed actions', () => { + expect(host.processMessage({ + source: iframe2.contentWindow, + origin: 'www.example.com', + data: 'amp-' + JSON.stringify({ + sentinel: '0-124', + type: 'full-overlay-frame', + }), + })).to.be.false; + }); + + it('should allow messages from frames with no whitelist for now', () => { + expect(host.processMessage({ + source: iframe3.contentWindow, + origin: 'www.example.com', + data: 'amp-' + JSON.stringify({ + sentinel: '0-125', + type: 'full-overlay-frame', + }), + })).to.be.true; + }); }); describe('send-positions', () => { @@ -357,8 +411,8 @@ describes.realWin('inabox-host:messaging', {}, env => { const frameMock = topWinMock.document.querySelectorAll()[0]; const expectedWin = sourceMock.parent.parent; host = new InaboxMessagingHost(win, [frameMock]); - expect(host.getFrameElement_(sourceMock, sentinel).contentWindow - ).to.deep.equal(expectedWin); + const {measurableFrame} = host.getFrameElement_(sourceMock, sentinel); + expect(measurableFrame.contentWindow).to.deep.equal(expectedWin); }); it('should return correct frame when all frames friendly', () => { @@ -367,8 +421,8 @@ describes.realWin('inabox-host:messaging', {}, env => { const frameMock = topWinMock.document.querySelectorAll()[0]; const expectedWin = sourceMock; host = new InaboxMessagingHost(win, [frameMock]); - expect(host.getFrameElement_(sourceMock, sentinel).contentWindow - ).to.deep.equal(expectedWin); + const {measurableFrame} = host.getFrameElement_(sourceMock, sentinel); + expect(measurableFrame.contentWindow).to.deep.equal(expectedWin); }); it('should return correct frame when many frames registered', () => { @@ -380,8 +434,8 @@ describes.realWin('inabox-host:messaging', {}, env => { const expectedWin = sourceMock; host = new InaboxMessagingHost( win, [frameMockWrong1, frameMockWrong2, frameMock]); - expect(host.getFrameElement_(sourceMock, sentinel).contentWindow - ).to.deep.equal(expectedWin); + const {measurableFrame} = host.getFrameElement_(sourceMock, sentinel); + expect(measurableFrame.contentWindow).to.deep.equal(expectedWin); }); it('should return cached frame', () => { @@ -392,8 +446,9 @@ describes.realWin('inabox-host:messaging', {}, env => { const creativeIframeMock = {}; host.iframeMap_[sentinel] = { 'iframe': creativeIframeMock, 'measurableFrame': creativeIframeMock}; - expect(host.getFrameElement_(creativeWinMock, sentinel) - ).to.equal(creativeIframeMock); + const {measurableFrame} = + host.getFrameElement_(creativeWinMock, sentinel); + expect(measurableFrame).to.equal(creativeIframeMock); }); it('should return null if frame is not registered', () => {