Skip to content

Commit

Permalink
Add message whitelist functionality to inabox messaging host (#18698)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
zombifier authored and keithwrightbos committed Nov 8, 2018
1 parent 4bef1f1 commit b7dec00
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 15 deletions.
20 changes: 14 additions & 6 deletions ads/inabox/inabox-messaging-host.js
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand Down
73 changes: 64 additions & 9 deletions test/functional/inabox/test-inabox-messaging-host.js
Expand Up @@ -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', () => {
Expand All @@ -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,
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand Down

0 comments on commit b7dec00

Please sign in to comment.