1766323 - JS Load Restrictions Telemetry still has prefcalls.js entries

Status: RESOLVED FIXED

(Firefox :: Security, defect)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

SELECT event_object,
       event_method,
       event_string_value,
       app_version,
       normalized_channel,
       TO_JSON_STRING(event_map_values),
       event_process,
       count(*) AS count_reports,
       count(distinct client_id) as count_distinct_clients
FROM telemetry.events
WHERE event_category = 'security'
  AND event_method = 'javascriptLoad'
  AND submission_date >= '2022-01-1'
  AND (
    --(normalized_channel = 'release' and app_version > '98') or
    (normalized_channel = 'beta' and app_version = '100.0') or
    (normalized_channel = 'nightly' and app_version = '101.0a1')
    )
GROUP BY event_method,
         event_object,
         event_string_value,
         app_version,
         normalized_channel,
         TO_JSON_STRING(event_map_values),
         event_process
ORDER BY app_version desc, normalized_channel, event_string_value

Telemetry shows prefcalls.js showing up in Beta (as well as http:// and https:// schemes (and one chrome:// scheme??)

I had hoped that Bug 1759887 might have fixed this (I know it fixed one very unusual corner case) but it seems we are back at the drawing board for how in the world prefcalls.js can show up. I'll update this bug with my understanding of why this should never happen....

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1766323: If the preference service is not initialized; don't say we checked hacks r?freddyb

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

prefCalls.js comes from nsReadConfig::readConfigFile().

nsReadConfig::readConfigFile() is only called after observing prefservice:before-read-userprefs", which is only sent in FinishInitializingUserPrefs(), which is only called by nsXREDirProvider::FinishInitializingUserPrefs() which is only called in XRE_mainRun(). XRE_mainRun is only called once during startup.

Now an important aspect of the above is that nsReadConfig is constructed (and Initialized, which is where it adds the observer) via a Component. That occurs in Preferences::GetInstanceForService() but crucially only if there is a value in general.config.filename.

Now remeber the event we were observing? prefservice:before-read-userprefs - BEFOREREAD!! That's a mistake, because that event is sent after we initial userprefs as seen in XRE_mainRun(). But let's set that aside.

---

Let's dive in a bit to Preference Initialization.

When Preferences::GetInstanceForService() gets constructed, it will call InitInitialObjects(startup = true). This calls StaticPrefs::InitAll(), then will read prefs from jar:$gre/omni.jar!/greprefs.js, jar:$gre/omni.jar!/defaults/pref/*.js (yes *), jar:$gre/omni.jar!/defaults/backgroundtasks/*.js, $gre/greprefs.js (this is outside the jar now, we're on the filesystem), and then $gre/defaults/pref/*.js (excepting macprefs.js/winpref.js/unix.js depending on platform). That last one is the most important - it's what loads pref files that were admin-provided by sticking them in the profile dir. You'd think we're done, but then it may also read jar:$app/omni.jar!/defaults/preferences/*.js, jar:$app/omni.jar!/defaults/backgroundtasks/*.js and all the pref files in NS_APP_PREFS_DEFAULTS_DIR_LIST although truthfully I'm not really sure what that last one really implies/is.

The important thing to see here is that when the PreferencesService is constructed, it reads default preferences (including admin preferences) but not user preferences and it only creates the readConfig() component if general.config.filename is set by one of those things.

So for prefCalls.js to get loaded, general.config.filename had to have been set.

---

Now let's change tacks and build up some background on this 'Javascript Load Telemetry'.

It happens over in ValidateScriptFile. This function gets called (via a pointer) in MaybeValidateFilename which is called when we load a script before we parse and execute it.

The most relevant bit of ValidateScriptFilename for our purposes right now is the JS Hacks stuff. We have a function DetectJsHacks which is used by a few functions in this file. It will check some preferences (most importantly general.config.filename!!) and if they are set, will set sJSHacksPresent to true, which will tell ValidateScriptFilename to let any script name through. If DetectJsHacks could not do its thing because it was called off-main-thread it will not set sJSHacksChecked and if that is not set, then ValidateScriptFilename will also let any script name through.

---

So we have events occuring in this sequence:

• Preferences::GetInstanceForService() gets called which
• ... reads a bunch of default and admin preference files but not user preference files
• ... checks the general.config.filename pref and if it's set creates the nsReadConfig class which starts listening for prefservice:before-read-userprefs
• User Preferences get read
• The event prefservice:before-read-userprefs gets fired
• ... causing nsReadConfig::readConfigFile() to get called
• nsReadConfig::readConfigFile() loads prefCalls.js
• ... which takes us into ValidateScriptFilename for the first time
• ... which checks general.config.filename and sees that it has a value
• ... which sets sJSHacksPresent
• ... which lets prefCalls.js through

Except we have telemetry for prefCalls.js! How!?

Here's the set of assertions that lead to this contradiction:

• prefCalls.js cannot be loaded unless general.config.filename is set in the default branch
• If general.config.filename is set in the default or user branch, we set sJSHacksPresent = True
• If sJSHacksPresent == true, we let anything through and don't do any telemetry

---

Okay, let's talk a little bit about what has caused telemetry to show up, or non-FF JS files to be loaded.

• general.config.filename (were it not checked and excluded) gets executed. So does autoadmin.global_config_url
• If you've jumped through the hoops to do legacy add-ons, such as this method then the JS files those addons/userscripts runs show up in the telemetry

---

Theories:

• prefCalls.js is being loaded some other way. In the Firefox code, AFAICT it can only be loaded in the way I described - during startup, right after initializing user prefs. But maybe it's being loaded by some different code?
• Something is triggering ValidateScriptFile before preferences initialization such that DetectJSHacks doesn't see any hacks but still sets sJSHacksChecked. Maybe the Preferences Service is not available? This is a good theory I don't currently check...

I'm going to pause analysis here and post that patch.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57e6ee5d925c
If the preference service is not initialized; don't say we checked hacks r=freddyb

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/57e6ee5d925c

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

This landed in Nightly 101; so if it was effective Beta 101 should show no telemetry.

As of today though, telemetry shows 2 total reports from 2 distinct clients with prefcalls.js. (And 103 reports from 3 distinct clients for 'https', and 1 report from 1 client for 'resourca'.)

Comment 6

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Cross-referencing whether general.config.filename is set (I forgot we report this in the telemetry environment) - it is set for the submissions with 'prefcalls.js' and is not set for the ones with 'https' and 'resourca'. Telemetry doesn't tell us if it is set on the default and then re-set or un-set on user branch; the code is kind of hard to digest but it's something....

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

It can't be a race condition. We only set sJSHacksChecked = true; on the main thread, and if it's not true, we let anything through.