Bug 1162476 - Telemetry should reject external pings with invalid types. r=vladan a=sledru
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -354,29 +354,16 @@
"description": "Geolocation on OS X is either MLS or CoreLocation"
},
"JS_DEPRECATED_LANGUAGE_EXTENSIONS_IN_CONTENT": {
"expires_in_version": "never",
"kind": "enumerated",
"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",
"description": "XUL cache was disabled"
},
"MEMORY_RESIDENT": {
"alert_emails": ["memshrink-telemetry-alerts@mozilla.com"],
"expires_in_version": "never",
@@ -4396,16 +4383,36 @@
},
"TELEMETRY_FILES_EVICTED": {
"alert_emails": ["perf-telemetry-alerts@mozilla.com", "rvitillo@mozilla.com"],
"expires_in_version": "never",
"kind": "enumerated",
"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",
"description": "a testing histogram; not meant to be touched"
},
"TELEMETRY_TEST_COUNT": {
"expires_in_version": "never",
"kind": "count",
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -198,16 +198,21 @@ this.TelemetryController = Object.freeze
},
/**
* Submit ping payloads to Telemetry. This will assemble a complete ping, adding
* environment data, client id and some general info.
* 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.
* @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
* id, false otherwise.
* @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
@@ -571,16 +576,25 @@ let Impl = {
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
* @returns {Promise} A promise that is resolved with the ping id once the ping is stored or sent.
*/
submitExternalPing: function send(aType, aPayload, aOptions) {
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);
// Always persist the pings if we are allowed to.
let archivePromise = TelemetryArchive.promiseArchivePing(pingData)
.catch(e => this._log.error("submitExternalPing - Failed to archive ping " + pingData.id, e));
let p = [ archivePromise ];
--- a/toolkit/components/telemetry/tests/unit/test_PingAPI.js
+++ b/toolkit/components/telemetry/tests/unit/test_PingAPI.js
@@ -165,16 +165,39 @@ add_task(function* test_clientId() {
id = yield TelemetryController.submitExternalPing("test-type", {}, {addClientId: true});
ping = yield TelemetryArchive.promiseArchivedPingById(id);
Assert.equal(ping.clientId, 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.");
+ }
+});
+
add_task(function* test_currentPingData() {
yield TelemetrySession.setup();
// Setup test data.
let h = Telemetry.getHistogramById("TELEMETRY_TEST_RELEASE_OPTOUT");
h.clear();
h.add(1);
let k = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT");