Closed Bug 1374745 Opened 2 years ago Closed Last year

nsScriptSecurityManager should not use a string bundle before profile selection

Categories

(Core :: DOM: Security, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: florian, Assigned: baku)

Details

(Whiteboard: [domsecurity-backlog1])

Attachments

(1 file)

The string bundle service is currently loaded very early during startup, before profile selection, due to the code at http://searchfox.org/mozilla-central/rev/2bcd258281da848311769281daf735601685de2d/caps/nsScriptSecurityManager.cpp#1342

This will result in strings in the wrong locale when a language pack is installed.

Given that this bundle is used only 3 times and for error cases, I don't think it should be loaded during startup. Instead we could either load it the first time it's used, or each time it's used.

Once this is fixed, we should blacklist the @mozilla.org/intl/stringbundle;1 service before profile selection in the browser_startup.js test.
I agree, we could just use load it the first time we use it. Putting in the backlog for now.
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
Attached patch bundle.patchSplinter Review
A simple helper class to create the bundle only when needed.
Assignee: nobody → amarchesini
Attachment #8973708 - Flags: review?(ckerschb)
Are we sure that this is the only place where stringbundle is used before profile selection?
Is there a way to check this? I can easily add @mozilla.org/intl/stringbundle;1 into the blacklist in browser_startup.js, but I want a confirm first.
Comment on attachment 8973708 [details] [diff] [review]
bundle.patch

Review of attachment 8973708 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. I think this is the only occurence and you can add @mozilla.org/intl/stringbundle;1 to the blacklist in browser_startup.js

::: caps/nsScriptSecurityManager.cpp
@@ +91,5 @@
> +  GetOrCreate()
> +  {
> +    // Already shutting down. Nothing should require the use of the string
> +    // bundle when shutting down.
> +    if (sShutdown) {

Can we add a MOZ_ASSERT() here? I agree, this should never happen, but just in case. Otherwise any potential console message would just not be logged without any kind of notification that something might be wrong.
Attachment #8973708 - Flags: review?(ckerschb) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dba34d8471d2
nsScriptSecurityManager should not use a string bundle before profile selection, r=ckerschb
> Thanks. I think this is the only occurence and you can add
> @mozilla.org/intl/stringbundle;1 to the blacklist in browser_startup.js

No. I cannot do this. There is something else using stringbundle before the profile loading.

GDB says:

#3  0x00007fffe3d14377 in mozilla::widget::WidgetUtils::GetBrandShortName (aBrandName=...) at /home/baku/Sources/m/foobar/src/widget/WidgetUtils.cpp:153
#4  0x00007fffe3d785d6 in nsAppShell::Init (this=0x9a84c0) at /home/baku/Sources/m/foobar/src/widget/gtk/nsAppShell.cpp:186
#5  0x00007fffe3daa3dc in nsAppShellInit () at /home/baku/Sources/m/foobar/src/widget/nsAppShellSingleton.h:45
https://hg.mozilla.org/mozilla-central/rev/dba34d8471d2
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.