709173 - updater.exe needs to be verified to be an updater, not just an executable signed by Mozilla

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Ian Melven :imelven]

the current implementation of the silent update Windows service (see bug 481815) does a check on the updater.exe before running it to make sure that it is an executable signed by Mozilla - this check doesn't verify that it's actually an updater.exe - it could be Thunderbird, Firefox, etc. 

one check suggested is to check a 'magic token' in the updater.exe which is essentially a string resource that only an updater.exe and no other signed product would contain. another approach suggested is to check the original file name field of the file metadata and verify that it is 'updater.exe' and not another product. 

see also bug 709158 which covers the case that it IS an updater.exe - but not necessarily the right product's/channel's/version of updater.exe

Marking [sg:moderate], please discuss here in the bug if you have other opinions of this rating.

Comment 1

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Added a string table value with a unique value.
Only Mozilla updater applications will have this string table item and value.
An update operation will not run from the service if the updater does not contain this magic string value.

Comment 2

[Ian Melven :imelven]

Comment on attachment 580716 [details] [diff] [review]
Patch v1.

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

looks good to me, one small comment about a possible refactor that doesn't affect the security concerns here.

::: toolkit/components/maintenanceservice/workmonitor.cpp
@@ +405,5 @@
> +
> +  if (updaterModule) {
> +    FreeLibrary(updaterModule);
> +  }
> +

nit: you could refactor the above, move the FreeLibrary inside the preceding
block, make the 2nd block an else of the first if (!updaterModule)

Comment 3

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2.

Thanks for the review Ian, implemented nits.

Comment 4

[Robert Strong (they/them - no direct email)]

Comment on attachment 581251 [details] [diff] [review]
Patch v2.

Looks good

Comment 5

[Brian R. Bondy [:bbondy]]

Attachment: Patch v3.

Rebased.

Comment 6

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/mozilla-central/rev/df4ba8bfc9ca

Comment 7

[u279076]

Is this something QA can verify?

Comment 8

[Brian R. Bondy [:bbondy]]

I think you can copy another signed executable that is not the updater and rename it to updater.exe.  Then try to do an update.  The maintenance service log should contain an error code 28 which is the identity error.  I'm not 100% certain those are the steps but please try and let me know if it doesn't work.

Comment 9

[Simona Badau, Desktop QA]

(In reply to Brian R. Bondy [:bbondy] from comment #8)
> I think you can copy another signed executable that is not the updater and
> rename it to updater.exe.  Then try to do an update.  The maintenance
> service log should contain an error code 28 which is the identity error. 
> I'm not 100% certain those are the steps but please try and let me know if
> it doesn't work.

Tried the above indications:
- renamed the crashreport.exe to updater.exe and after the update was done the Crash Reporter Error dialog was displayed - the maintenance service log was not generated.
- renamed FirefoxSetup.exe to updater.exe and updated, after the restart the Install dialog was opened - the maintenance service log was not generated.

Also, replaced the Firefox updater with the Thunderbird one, the update was done the old way and the maintenance service log was not generated. 

Brian, any other ideas?

Comment 10

[Brian R. Bondy [:bbondy]]

ah, that's because the unelevated updater.exe needs to execute first in order to start the maintenance service operation.  I'm not sure how else you could reproduce this short of making a custom mozilla signed exe.

Comment 11

[Brian R. Bondy [:bbondy]]

I think relying on the log message alone is enough to test this task.

So make sure this is not present in the log after an update:
The updater.exe identity string is not valid.

And make sure this is present in the log:
The updater.exe application contains the Mozilla updater identity.

Comment 12

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0

Verified (on Win 7 and Win Vista x64)by checking the maintenanceservice logs that after updating Firefox 13 beta 4 to Firefox 13 beta 5 that the updater.exe is verified to be an updater and is signed by Mozilla: 

- "The updater.exe identity string is not valid" - is not present in the logs
and
- "The updater.exe application contains the Mozilla updater identity" - is present in the logs.

Thanks Brian for the guidance.