393823 - -osint logic not written out to the windows registry when using software update on the 1.8.0 branch

Status: RESOLVED FIXED

(Thunderbird :: Security, defect)

Description

[Scott MacGregor]

spinning out from Bug 389613. 

Stephen and I actually put a bunch of comments in Bug 389613 over the weekend, but we really should have broken this issue out into a new bug for triage purposes so I'm doing that now.

"Stephen identified a problem with this fix. On the 1.5.0.x branch, we compare
the application path and nothing else when determining if the app is still the
default. Thus, a user who is running 1.5.0.12 as the default and gets upgraded
to 1.5.0.13, won't get the osint flag written out to the registry for the
appropriate keys. You need to unset, then set Thunderbird as the default mail
client for the keys to get written out.

This isn't a problem when testing:

1.5.0.12 and 1.5.0.13 using clean VMs for each test.
1.5.0.12 and a debug 1.5.0.13 with the fix.

Note: the behavior of detecting default client status based on the application
path and not the command line arguments as well has changed on the 1.8.1 branch
so this isn't an issue there. It's specific to 1.8.0."

Comment 1

[Scott MacGregor]

Attachment: brute force fix

Same comment I put in Bug 389613:

"Here's a fix that seems to address the spot where the default client status
isn't changing, causing us to fail to write out the osint flags.

It's a brute force approach for sure, but this is for the 1.8.0 branch only,
and this approach has a lot less risk than modifying the different ways in
which we detect default app status for mailto, news, nntp, snews and feed urls.

Every time 1.5.0.x starts up, for each protocol the current application
instance is the default for, re-write out the registration keys. And re-write
out the protocol key giving us default status with the OS."

Comment 2

[Scott MacGregor]

Repeating the testing comments over in: https://bugzilla.mozilla.org/show_bug.cgi?id=389613#c49

Here's what I did to test the brute force patch:

(either a clean VM or make sure the mailto, nntp, news, feed and snews keys
don't have -osint already in the registry)

1) Downloaded a nightly 1.5.0.13pre build from before the original fix went in
like this one:
ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2007-07-31-06-mozilla1.8.0/thunderbird-1.5.0.13pre.en-US.win32.installer.exe

2) Make sure this installation is the default mail, news and feed client.

3) Verify that the test case still fails and the profile manager comes up.

4) Now check for updates and get updated to the latest update.

5) Restart to install the update

6) Quit thunderbird

7) you can manually inspect HKLM\mailto, HKLM\news, etc. to see that the -osint
is now written out into the registry.

8) Run the test case again and verify that nothing comes up.

Comment 3

[Scott MacGregor]

and since I temporarily checked in the brute force patch onto the branch so we could test it this weekend, here are Stephen's findings copied from https://bugzilla.mozilla.org/show_bug.cgi?id=389613#c50:

I can *finally*, *truly* verify this bug as fixed, using Scott's instructions
in comment 49, above.

C:\PROGRA~1\MOZILL~1\THUNDE~1.EXE -osint -compose %1 gets set under:

My Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Clients\Mail\Mozilla
Thunderbird\protocols\mailto\shell\open\command

Using the pre-fix build (2007-07-31-06 of mozilla1.8.0), I see the Profile
Manager invoked when running the testcase in comment 43.  Post-fix, after
upgrading to 2007-08-26-04, I can see that not even the compose window comes
up, which I think is the expected outcome given Scott's step #8 in comment 49.

Comment 4

[Daniel Veditz [:dveditz]]

Does this hack need to go into Tbird 2.0 as well, or is it just an issue with 1.5-era installers?

Comment 5

[Scott MacGregor]

This is just for 1.5 Dan. 

Comment 6

[Scott MacGregor]

Comment on attachment 278375 [details] [diff] [review]
brute force fix

asking David for an sr on this brute force patch which is currently on the 1.8.0.x branch to make it easier for QA and others to help verify the fix.

Comment 7

[Daniel Veditz [:dveditz]]

Looks like the "temporary" landing is still there, and we're going to take it anyway so marking fixed on the branch.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 278375 [details] [diff] [review]
brute force fix

approved for 1.8.0.14, a=dveditz for release-drivers