Tighten the types of JSON.stringify and postMessage. (#9936)

The new type limits the APIs to be used with `JsonObject` to enforce that objects being passed to them used quoted property names not subject to obfuscation.

Part of #9876

3p/ampcontext-integration.js

@@ -16,6 +16,7 @@
 import {AbstractAmpContext} from './ampcontext';
 import {computeInMasterFrame} from './3p';
 import {dev, user} from '../src/log';
+import {dict} from '../src/utils/object';
 
 
 /**
@@ -101,7 +102,7 @@ export class IntegrationAmpContext extends AbstractAmpContext {
   }
 
   /**
-   * @param {{width, height}=} opt_data
+   * @param {!JsonObject=} opt_data Fields: width, height
    */
   renderStart(opt_data) {
     this.client_.sendMessage('render-start', opt_data);
@@ -119,9 +120,9 @@ export class IntegrationAmpContext extends AbstractAmpContext {
    * @param {string} entityId See comment above for content.
    */
   reportRenderedEntityIdentifier(entityId) {
-    this.client_.sendMessage('entity-id', {
-      id: user().assertString(entityId),
-    });
+    this.client_.sendMessage('entity-id', dict({
+      'id': user().assertString(entityId),
+    }));
   }
 
   /**

3p/ampcontext.js

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import {dict} from '../src/utils/object';
 import {dev} from '../src/log';
 import {IframeMessagingClient} from './iframe-messaging-client';
 import {MessageType} from '../src/3p-frame-messaging';
@@ -178,7 +179,10 @@ export class AbstractAmpContext {
    *  @param {number} height The new height for the ad we are requesting.
    */
   requestResize(width, height) {
-    this.client_.sendMessage(MessageType.EMBED_SIZE, {width, height});
+    this.client_.sendMessage(MessageType.EMBED_SIZE, dict({
+      'width': width,
+      'height': height,
+    }));
   };
 
   /**

3p/iframe-messaging-client.js

@@ -79,7 +79,7 @@ export class IframeMessagingClient {
   /**
    *  Send a postMessage to Host Window
    *  @param {string} type The type of message to send.
-   *  @param {Object=} opt_payload The payload of message to send.
+   *  @param {JsonObject=} opt_payload The payload of message to send.
    */
   sendMessage(type, opt_payload) {
     this.hostWindow_.postMessage/*OK*/(

3p/integration.js

@@ -41,6 +41,7 @@ import {urls} from '../src/config';
 import {endsWith} from '../src/string';
 import {parseUrl, getSourceUrl, isProxyOrigin} from '../src/url';
 import {dev, initLogConstructor, setReportError, user} from '../src/log';
+import {dict} from '../src/utils/object.js';
 import {getMode} from '../src/mode';
 import {startsWith} from '../src/string.js';
 
@@ -546,15 +547,21 @@ function triggerNoContentAvailable() {
 }
 
 function triggerDimensions(width, height) {
-  nonSensitiveDataPostMessage('embed-size', {width, height});
+  nonSensitiveDataPostMessage('embed-size', dict({
+    'width': width,
+    'height': height,
+  }));
 }
 
 function triggerResizeRequest(width, height) {
-  nonSensitiveDataPostMessage('embed-size', {width, height});
+  nonSensitiveDataPostMessage('embed-size', dict({
+    'width': width,
+    'height': height,
+  }));
 }
 
 /**
- * @param {{width, height}=} opt_data
+ * @param {!JsonObject=} opt_data fields: width, height
  */
 function triggerRenderStart(opt_data) {
   nonSensitiveDataPostMessage('render-start', opt_data);
@@ -574,7 +581,11 @@ let currentMessageId = 0;
  */
 function getHtml(selector, attributes, callback) {
   const messageId = currentMessageId++;
-  nonSensitiveDataPostMessage('get-html', {selector, attributes, messageId});
+  nonSensitiveDataPostMessage('get-html', dict({
+    'selector': selector,
+    'attributes': attributes,
+    'messageId': messageId,
+  }));
 
   const unlisten = listenParent(window, 'get-html-result', data => {
     if (data.messageId === messageId) {
@@ -658,9 +669,9 @@ function onResizeDenied(observerCallback) {
 function reportRenderedEntityIdentifier(entityId) {
   user().assert(typeof entityId == 'string',
       'entityId should be a string %s', entityId);
-  nonSensitiveDataPostMessage('entity-id', {
-    id: entityId,
-  });
+  nonSensitiveDataPostMessage('entity-id', dict({
+    'id': entityId,
+  }));
 }
 
 /**

3p/messaging.js

@@ -17,15 +17,15 @@
 /**
  * Send messages to parent frame. These should not contain user data.
  * @param {string} type Type of messages
- * @param {*=} opt_object Data for the message.
+ * @param {!JsonObject=} opt_object Data for the message.
  */
 export function nonSensitiveDataPostMessage(type, opt_object) {
   if (window.parent == window) {
     return;  // Nothing to do.
   }
-  const object = opt_object || {};
-  object.type = type;
-  object.sentinel = window.context.sentinel;
+  const object = opt_object || /** @type {JsonObject} */ ({});
+  object['type'] = type;
+  object['sentinel'] = window.context.sentinel;
   window.parent./*OK*/postMessage(object,
       window.context.location.origin);
 }

ads/alp/handler.js

@@ -20,6 +20,7 @@ import {
 } from '../../src/url';
 import {closest, openWindowDialog} from '../../src/dom';
 import {dev} from '../../src/log';
+import {dict} from '../../src/utils/object';
 import {urls} from '../../src/config';
 import {isProxyOrigin, isLocalhostOrigin, parseUrl} from '../../src/url';
 import {startsWith} from '../../src/string';
@@ -149,9 +150,9 @@ function navigateTo(win, a, url) {
   const target = (a.target || '_top').toLowerCase();
   const a2aAncestor = getA2AAncestor(win);
   if (a2aAncestor) {
-    a2aAncestor.win./*OK*/postMessage('a2a;' + JSON.stringify({
-      url,
-    }), a2aAncestor.origin);
+    a2aAncestor.win./*OK*/postMessage('a2a;' + JSON.stringify(dict({
+      'url': url,
+    })), a2aAncestor.origin);
     return;
   }
   openWindowDialog(win, url, target);

ads/inabox/inabox-messaging-host.js

@@ -21,6 +21,7 @@ import {
   MessageType,
 } from '../../src/3p-frame-messaging';
 import {dev} from '../../src/log';
+import {dict} from '../../src/utils/object';
 import {expandFrame, collapseFrame} from './frame-overlay-helper';
 
 /** @const */
@@ -153,7 +154,7 @@ export class InaboxMessagingHost {
           serializeMessage(
               MessageType.FULL_OVERLAY_FRAME_RESPONSE,
               request.sentinel,
-              {success: true}),
+              dict({'success': true})),
           origin);
     });
 
@@ -173,7 +174,7 @@ export class InaboxMessagingHost {
           serializeMessage(
               MessageType.CANCEL_FULL_OVERLAY_FRAME_RESPONSE,
               request.sentinel,
-              {success: true}),
+              dict({'success': true})),
           origin);
     });
 

build-system/conformance-config.textproto

@@ -154,3 +154,26 @@ requirement: {
   error_message: 'Symbol is not allowed'
   value: 'Symbol'
 }
+
+requirement: {
+  type: RESTRICTED_METHOD_CALL
+  error_message: 'postMessage must be called with a string or a JsonObject'
+  value: 'Window.prototype.postMessage:function((string|?JsonObject), string)'
+  # Guaranteed same version call
+  whitelist: 'src/web-worker/web-worker.js'
+  # Allowing violations in ads code for now.
+  # We would have to fix this to property obfuscated the 3p frame code.
+  whitelist: 'ads/google/doubleclick.js'
+  whitelist: 'ads/google/imaVideo.js'
+}
+
+requirement: {
+  type: RESTRICTED_NAME_CALL
+  error_message: 'JSON.stringify must be called with a JsonObject'
+  # Unfortunately the Array is untyped, because the compiler doesn't check
+  # for the template type.
+  value: 'JSON.stringify:function((?JsonObject|AmpViewerMessage|string|number|boolean|undefined|Array),!Function=)'
+  # Allowing violations in ads code for now.
+  # We would have to fix this to property obfuscated the 3p frame code.
+  whitelist: 'ads/google/doubleclick.js'
+}

extensions/amp-accordion/0.1/amp-accordion.js

@@ -20,7 +20,7 @@ import {Layout} from '../../../src/layout';
 import {closest} from '../../../src/dom';
 import {dev, user} from '../../../src/log';
 import {removeFragment} from '../../../src/url';
-import {map} from '../../../src/utils/object';
+import {dict} from '../../../src/utils/object';
 import {tryFocus} from '../../../src/dom';
 
 class AmpAccordion extends AMP.BaseElement {
@@ -35,7 +35,7 @@ class AmpAccordion extends AMP.BaseElement {
     /** @private {?string} */
     this.sessionId_ = null;
 
-    /** @private {?Object<string,boolean>} */
+    /** @private {?JsonObject} */
     this.currentState_ = null;
 
     /** @private {boolean} */
@@ -113,23 +113,24 @@ class AmpAccordion extends AMP.BaseElement {
 
   /**
    * Get previous state from sessionStorage.
-   * @return {!Object}
+   * @return {!JsonObject}
    * @private
    */
   getSessionState_() {
     if (this.sessionOptOut_) {
-      return map();
+      return dict();
     }
     try {
       const sessionStr =
           this.win./*OK*/sessionStorage.getItem(
               dev().assertString(this.sessionId_));
       return sessionStr
-          ? /** @type {!Object} */ (JSON.parse(dev().assertString(sessionStr)))
-          : map();
+          ? /** @type {!JsonObject} */ (
+              JSON.parse(dev().assertString(sessionStr)))
+          : dict();
     } catch (e) {
       dev().fine('AMP-ACCORDION', e.message, e.stack);
-      return map();
+      return dict();
     }
   }
 

extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js

@@ -26,6 +26,7 @@ import {
 } from '../../../src/iframe-helper';
 import {viewerForDoc} from '../../../src/services';
 import {dev} from '../../../src/log';
+import {dict} from '../../../src/utils/object';
 import {timerFor} from '../../../src/services';
 import {setStyle} from '../../../src/style';
 import {loadPromise} from '../../../src/event-helper';
@@ -122,9 +123,11 @@ export class AmpAdXOriginIframeHandler {
           this.positionObserver_.observe(
               dev().assertElement(this.iframe),
               PositionObserverFidelity.HIGH, pos => {
+                // Valid cast because it is an external object.
+                const posCast = /** @type {!JsonObject} */ (pos);
                 this.positionObserverHighFidelityApi_.send(
                     POSITION_HIGH_FIDELITY,
-                    pos);
+                    posCast);
               });
         });
     }
@@ -151,7 +154,10 @@ export class AmpAdXOriginIframeHandler {
 
           postMessageToWindows(
               this.iframe, [{win: source, origin}],
-              'get-html-result', {content, messageId}, true
+              'get-html-result', dict({
+                'content': content,
+                'messageId': messageId,
+              }), true
           );
         }, true, false));
 
@@ -385,7 +391,10 @@ export class AmpAdXOriginIframeHandler {
         this.iframe,
         [{win: source, origin}],
         success ? 'embed-size-changed' : 'embed-size-denied',
-        {requestedWidth, requestedHeight},
+        dict({
+          'requestedWidth': requestedWidth,
+          'requestedHeight': requestedHeight,
+        }),
         true);
   }
 
@@ -397,10 +406,10 @@ export class AmpAdXOriginIframeHandler {
     if (!this.embedStateApi_) {
       return;
     }
-    this.embedStateApi_.send('embed-state', {
-      inViewport,
-      pageHidden: !this.viewer_.isVisible(),
-    });
+    this.embedStateApi_.send('embed-state', dict({
+      'inViewport': inViewport,
+      'pageHidden': !this.viewer_.isVisible(),
+    }));
   }
 
   /**

extensions/amp-analytics/0.1/cid-impl.js

@@ -32,6 +32,7 @@ import {
   isProxyOrigin,
   parseUrl,
 } from '../../../src/url';
+import {dict} from '../../../src/utils/object';
 import {isIframed} from '../../../src/dom';
 import {getCryptoRandomBytesArray} from '../../../src/utils/bytes';
 import {viewerForDoc} from '../../../src/services';
@@ -315,10 +316,10 @@ export function viewerBaseCid(ampdoc, opt_data) {
           // For backward compatibility: #4029
           if (data && !tryParseJson(data)) {
             // TODO(dvoytenko, #9019): use this for reporting: dev().error('cid', 'invalid cid format');
-            return JSON.stringify({
-              time: Date.now(), // CID returned from old API is always fresh
-              cid: data,
-            });
+            return JSON.stringify(dict({
+              'time': Date.now(), // CID returned from old API is always fresh
+              'cid': data,
+            }));
           }
           return data;
         });
@@ -340,10 +341,10 @@ export function viewerBaseCid(ampdoc, opt_data) {
  * @return {string}
  */
 function createCidData(cidString) {
-  return JSON.stringify({
-    time: Date.now(),
-    cid: cidString,
-  });
+  return JSON.stringify(dict({
+    'time': Date.now(),
+    'cid': cidString,
+  }));
 }
 
 /**

extensions/amp-bind/0.1/bind-expression.js

@@ -389,8 +389,10 @@ export class BindExpression {
    * @private
    */
   memberAccessWarning_(target, member) {
-    user().warn(TAG, `Cannot read property ${JSON.stringify(member)} of ` +
-        `${JSON.stringify(target)}; returning null.`);
+    // Cast valid, because we don't care for the logging.
+    const stringified = JSON.stringify(/** @type {!JsonObject} */ (member));
+    user().warn(TAG, `Cannot read property ${stringified} of ` +
+        `${stringified}; returning null.`);
   }
 
   /**

extensions/amp-bind/0.1/bind-impl.js

@@ -23,7 +23,7 @@ import {
 } from '../../../src/services';
 import {chunk, ChunkPriority} from '../../../src/chunk';
 import {dev, user} from '../../../src/log';
-import {deepMerge} from '../../../src/utils/object';
+import {dict, deepMerge} from '../../../src/utils/object';
 import {getMode} from '../../../src/mode';
 import {filterSplice} from '../../../src/utils/array';
 import {installServiceInEmbedScope} from '../../../src/service';
@@ -130,8 +130,8 @@ export class Bind {
     /** @const @private {!./bind-validator.BindValidator} */
     this.validator_ = new BindValidator();
 
-    /** @const @private {!Object} */
-    this.scope_ = Object.create(null);
+    /** @const @private {!JsonObject} */
+    this.scope_ = dict();
 
     /** @const @private {!../../../src/service/resources-impl.Resources} */
     this.resources_ = resourcesForDoc(ampdoc);
@@ -617,7 +617,7 @@ export class Bind {
         return;
       }
       const promise = this.resources_.mutateElement(element, () => {
-        const mutations = {};
+        const mutations = dict();
         let width, height;
 
         updates.forEach(update => {