Bug 1162476 - Telemetry should reject external pings with invalid types. r=vladan

toolkit/components/telemetry/Histograms.json

@@ -359,19 +359,6 @@
     "n_values": 10,
     "description": "Use of SpiderMonkey's deprecated language extensions in web content: ForEach=0, DestructuringForIn=1, LegacyGenerator=2, ExpressionClosure=3, LetBlock=4, LetExpression=5, NoSuchMethod=6, FlagsArgument=7, RegExpSourceProp=8"
   },
-  "TELEMETRY_PING": {
-    "expires_in_version": "never",
-    "kind": "exponential",
-    "high": "3000",
-    "n_buckets": 10,
-    "extended_statistics_ok": true,
-    "description": "Time taken to submit telemetry info (ms)"
-  },
-  "TELEMETRY_SUCCESS": {
-    "expires_in_version": "never",
-    "kind": "boolean",
-    "description": "Successful telemetry submission"
-  },
   "XUL_CACHE_DISABLED": {
     "expires_in_version": "default",
     "kind": "flag",
@@ -4401,6 +4388,26 @@
     "n_values": 30,
     "description": "Number of telemetry pings evicted at startup"
   },
+  "TELEMETRY_PING": {
+    "expires_in_version": "never",
+    "kind": "exponential",
+    "high": "3000",
+    "n_buckets": 10,
+    "extended_statistics_ok": true,
+    "description": "Time taken to submit telemetry info (ms)"
+  },
+  "TELEMETRY_SUCCESS": {
+    "expires_in_version": "never",
+    "kind": "boolean",
+    "description": "Successful telemetry submission"
+  },
+  "TELEMETRY_INVALID_PING_TYPE_SUBMITTED": {
+    "alert_emails": ["telemetry-client-dev@mozilla.com"],
+    "expires_in_version": "never",
+    "kind": "count",
+    "keyed": true,
+    "description": "Count of individual invalid ping types that were submitted to Telemetry."
+  },
   "TELEMETRY_TEST_FLAG": {
     "expires_in_version": "never",
     "kind": "flag",

toolkit/components/telemetry/TelemetryController.jsm

@@ -195,6 +195,11 @@ this.TelemetryController = Object.freeze({
    * Depending on configuration, the ping will be sent to the server (immediately or later)
    * and archived locally.
    *
+   * To identify the different pings and to be able to query them pings have a type.
+   * A type is a string identifier that should be unique to the type ping that is being submitted,
+   * it should only contain alphanumeric characters and '-' for separation, i.e. satisfy:
+   * /^[a-z0-9][a-z0-9-]+[a-z0-9]$/i
+   *
    * @param {String} aType The type of the ping.
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} [aOptions] Options object.
@@ -558,6 +563,15 @@ let Impl = {
     this._log.trace("submitExternalPing - type: " + aType + ", server: " + this._server +
                     ", aOptions: " + JSON.stringify(aOptions));
 
+    // Enforce the type string to only contain sane characters.
+    const typeUuid = /^[a-z0-9][a-z0-9-]+[a-z0-9]$/i;
+    if (!typeUuid.test(aType)) {
+      this._log.error("submitExternalPing - invalid ping type: " + aType);
+      let histogram = Telemetry.getKeyedHistogramById("TELEMETRY_INVALID_PING_TYPE_SUBMITTED");
+      histogram.add(aType, 1);
+      return Promise.reject(new Error("Invalid type string submitted."));
+    }
+
     const pingData = this.assemblePing(aType, aPayload, aOptions);
     this._log.trace("submitExternalPing - ping assembled, id: " + pingData.id);
 

toolkit/components/telemetry/tests/unit/test_PingAPI.js

@@ -17,6 +17,7 @@ XPCOMUtils.defineLazyGetter(this, "gPingsArchivePath", function() {
   return OS.Path.join(OS.Constants.Path.profileDir, "datareporting", "archived");
 });
 
+const Telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry);
 
 function run_test() {
   do_get_profile(true);
@@ -165,3 +166,26 @@ add_task(function* test_clientId() {
   // Finish setup.
   yield promiseSetup;
 });
+
+add_task(function* test_InvalidPingType() {
+  const TYPES = [
+    "a",
+    "-",
+    "¿€€€?",
+    "-foo-",
+    "-moo",
+    "zoo-",
+    ".bar",
+    "asfd.asdf",
+  ];
+
+  for (let type of TYPES) {
+    let histogram = Telemetry.getKeyedHistogramById("TELEMETRY_INVALID_PING_TYPE_SUBMITTED");
+    Assert.equal(histogram.snapshot(type).sum, 0,
+                 "Should not have counted this invalid ping yet: " + type);
+    Assert.ok(promiseRejects(TelemetryController.submitExternalPing(type, {})),
+              "Ping type should have been rejected.");
+    Assert.equal(histogram.snapshot(type).sum, 1,
+                 "Should have counted this as an invalid ping type.");
+  }
+});