Bug 1586920 - Strip non-chrome/resource URLs from BHR r=mconley
authorDoug Thayer <dothayer@mozilla.com>
Wed, 13 Nov 2019 21:07:07 +0000
changeset 501847 60db8c551c38d0619df981ac11e5616b8d8e3002
parent 501846 bd28a94055a30b1bb460d2735407ffa97c3fbcba
child 501848 f0968dabe1ff713ae3f28861dae1ca6524aafd80
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1586920
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1586920 - Strip non-chrome/resource URLs from BHR r=mconley This is just a precaution - I can't actually observe any instances of the dynamic strings added in the parent revision including anything other than chrome or resource URLs. However, since we are including URLs and since there's no hard guarantees that we won't accidentally cause, say, a file:// URL to make it in here, it seems like a reasonable precaution to strip them out. Differential Revision: https://phabricator.services.mozilla.com/D52114
toolkit/components/backgroundhangmonitor/BHRTelemetryService.jsm
--- a/toolkit/components/backgroundhangmonitor/BHRTelemetryService.jsm
+++ b/toolkit/components/backgroundhangmonitor/BHRTelemetryService.jsm
@@ -64,16 +64,32 @@ BHRTelemetryService.prototype = Object.f
       return idx;
     });
 
     // Native stack frames are [modIdx, offset] arrays. If we have a valid
     // module index, we want to map it to the this.payload.modules array.
     for (let i = 0; i < stack.length; ++i) {
       if (Array.isArray(stack[i]) && stack[i][0] !== -1) {
         stack[i][0] = moduleIdxs[stack[i][0]];
+      } else if (typeof stack[i] == "string") {
+        // This is just a precaution - we don't currently know of sensitive
+        // URLs being included in label frames' dynamic strings which we
+        // include here, but this is just an added guard. Here we strip any
+        // string with a :// in it that isn't a chrome:// or resource://
+        // URL. This is not completely robust, but we are already trying to
+        // protect against this by only including dynamic strings from the
+        // opt-in AUTO_PROFILER_..._NONSENSITIVE macros.
+        let match = /[^\s]+:\/\/.*/.exec(stack[i]);
+        if (
+          match &&
+          !match[0].startsWith("chrome://") &&
+          !match[0].startsWith("resource://")
+        ) {
+          stack[i] = stack[i].replace(match[0], "(excluded)");
+        }
       }
     }
 
     // Create the hang object to record in the payload.
     this.payload.hangs.push({
       duration,
       thread,
       runnableName,