🐛 verify message source on cached sentinel in inabox host

ads/inabox/inabox-messaging-host.js

@@ -56,6 +56,7 @@ class NamedObservable {
 /** @typedef {{
       iframe: !HTMLIFrameElement,
       measurableFrame: !HTMLIFrameElement,
+      source: !Window,
       observeUnregisterFn: (!UnlistenDef|undefined),
   }} */
 let AdFrameDef;
@@ -269,8 +270,12 @@ export class InaboxMessagingHost {
    * @private
    */
   getFrameElement_(source, sentinel) {
-    if (this.iframeMap_[sentinel]) {
-      return this.iframeMap_[sentinel];
+    const knownFrame = this.iframeMap_[sentinel];
+    if (knownFrame) {
+      // A sentinel is bound to the source window that first registered it.
+      // Reject a message that reuses the sentinel from a different source so
+      // a frame can't read another frame's position by spoofing its sentinel.
+      return knownFrame.source === source ? knownFrame : null;
     }
     const measurableFrame = this.getMeasureableFrame(source);
     if (!measurableFrame) {
@@ -285,7 +290,7 @@ export class InaboxMessagingHost {
         j++, tempWin = tempWin.parent
       ) {
         if (iframe.contentWindow == tempWin) {
-          this.iframeMap_[sentinel] = {iframe, measurableFrame};
+          this.iframeMap_[sentinel] = {iframe, measurableFrame, source};
           return this.iframeMap_[sentinel];
         }
         if (tempWin == window.top) {

test/unit/inabox/test-inabox-messaging-host.js

@@ -103,6 +103,35 @@ describes.realWin('inabox-host:messaging', {}, (env) => {
       ).to.be.false;
     });
 
+    it('should not leak position to a frame reusing another sentinel', () => {
+      // iframe1 registers the sentinel.
+      expect(
+        host.processMessage({
+          source: iframe1.contentWindow,
+          origin: 'www.example.com',
+          data:
+            'amp-' +
+            JSON.stringify({
+              sentinel: '0-123',
+              type: 'send-positions',
+            }),
+        })
+      ).to.be.true;
+      // A different frame reusing iframe1's sentinel is rejected.
+      expect(
+        host.processMessage({
+          source: iframe2.contentWindow,
+          origin: 'www.evil.com',
+          data:
+            'amp-' +
+            JSON.stringify({
+              sentinel: '0-123',
+              type: 'send-positions',
+            }),
+        })
+      ).to.be.false;
+    });
+
     it('should ignore message from untrusted iframe', () => {
       expect(
         host.processMessage({
@@ -511,6 +540,7 @@ describes.realWin('inabox-host:messaging', {}, (env) => {
       host.iframeMap_[sentinel] = {
         'iframe': creativeIframeMock,
         'measurableFrame': creativeIframeMock,
+        'source': creativeWinMock,
       };
       const {measurableFrame} = host.getFrameElement_(
         creativeWinMock,
@@ -519,6 +549,21 @@ describes.realWin('inabox-host:messaging', {}, (env) => {
       expect(measurableFrame).to.equal(creativeIframeMock);
     });
 
+    it('should not return cached frame for a different source', () => {
+      host.getMeasureableFrame = () => {
+        throw new Error('Error!!');
+      };
+      const creativeWinMock = {};
+      const otherWinMock = {};
+      const creativeIframeMock = {};
+      host.iframeMap_[sentinel] = {
+        'iframe': creativeIframeMock,
+        'measurableFrame': creativeIframeMock,
+        'source': creativeWinMock,
+      };
+      expect(host.getFrameElement_(otherWinMock, sentinel)).to.be.null;
+    });
+
     it('should return null if frame is not registered', () => {
       const iframeObj = createNestedIframeMocks(6, 3);
       const sourceMock = iframeObj.source;