modified Signed extensions appear to install (but don't). Report the error

RESOLVED FIXED in mozilla1.8final

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: Alex Sirota (iosart), Assigned: rstrong)

Tracking

Trunk
mozilla1.8final
x86
Windows XP
Points:
---
Bug Flags:
blocking-aviary1.5 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse])

Attachments

(1 attachment)

(Reporter)

Description

13 years ago
The tests here:
http://www.mozilla.org/projects/xpinstall/signed/testcases/index.html fail.

Meaning that an original signed extension appears signed, and so does the
altered version. 

Another test - download a signed extension from here:
http://www.j-maxx.net/abtrans/abextension.php, modify a file inside the
extension keeping the cert. (metadata) intact. The extension still appears signed.

IMHO this is a serious security bug - a user can be installing a malware
believing it is signed by an entity he/she trusts.
(Reporter)

Comment 1

13 years ago
I tested this a bit further, and it seems that this is an extension manager bug
with the latest FF nightly.

If you press "Install" it actually says "Install success". 
After you restart FF, the extension appears in EM, but without its full name -
only the filename that was downloaded. 

The extension seems -not- to be installed - it just sits there. So, it now looks
more like an EM bug than a security one. 
firefox 1.0 does the right thing (almost). A dialog box appears saying it
couldn't _download_ the file (wrong, it downloaded) because "Signing not
verified" (correct). While the alert is showing the extension appears in the EM
dialog behind as "Install Success" (wrong), but disappears from the EM dialog
when you click OK on the error alert (correct in the end).

Over to the Firefox product, it's the front end that's wrong.
Assignee: xpi-engine → bugs
Component: Installer: XPInstall Engine → Extension/Theme Manager
Product: Core → Firefox
QA Contact: bugs
Summary: Signed extensions still appear signed after being altered → modified Signed extensions appear to install (but don't actually)
Clearing the security flag so more people who could fix the UI might see it.
Group: security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-aviary1.1?
Summary: modified Signed extensions appear to install (but don't actually) → modified Signed extensions appear to install (but don't). Report the error
Whiteboard: [sg:nse]
Flags: blocking-aviary1.1? → blocking-aviary1.1+
When installing the extension from http://www.j-maxx.net/abtrans/abextension.php
it displayed as signed and installed successfully when unmodified. I then
downloaded it, modified the install.rdf inside the xpi, and tried to install it.
The ui showed as signed and the extension failed to install with an alert stating:
Deer Park could not download the file at
<path to file>/albhed-0.1.2.xpi
because: Signing could not be verified.

This was reported to the js console:
Signature Verification Error: the signature on install.rdf is invalid because
the MANIFEST.MF file does not contain a valid hash of the file being verified.

The extension was also removed from the ui after it failed to install.

This is WFM but I am going to hold off on resolving since this may not be
entirely the desired behavior though I believe it is close enough. Also, I
suspect that the patch from bug 286034 may have fixed this bug.
This is basically correct behavior. The only remaining problem is unrelated to
signing: when there's an installation error the message starts out "Firefox
could not download the file at" when it's quite often not a download error.

Would be better to say "Firefox could not install the file", and for the few
times it is a download problem finish with "because: download error". (Right now
it will say it could not download the file because of download error, which is
annoying.) The message issue is separate and probably already filed, although I
couldn't find one.

The string is
http://lxr.mozilla.org/mozilla/source/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties#64

The code is
http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/content/extensions.js#361

The code doesn't handle status 999 correctly -- that's not an install failure,
that's a success with restart required to finish replacing in-use files.
Thanks Daniel - I have a couple of other patches in progress including one that
handles the status 999 and will fix this after the freeze is lifted.
Created attachment 184718 [details] [diff] [review]
patch

I split this out from another patch. Per bsmedberg I changed
errorInstallMessage to errorInstallMsg to make it easier for translators to see
there is a change. I test for 999 instead of just going with all errors < 0
since there may be other errors above 0 that should be displayed though I
suspect this is unlikely.
Assignee: bugs → moz_bugzilla
Status: NEW → ASSIGNED
Attachment #184718 - Flags: review?(dveditz)
I doubt that a safer patch could be found to fix the remaining issues so
nominating for 1.8b2
Flags: blocking1.8b2?
Comment on attachment 184718 [details] [diff] [review]
patch

Benjamin - this is a very simple / safe patch that fixes the remaining issues
as pointed out by dveditz and with blockers that were just added for 1.8b2 I
think that it may be appropriate to land this.
Attachment #184718 - Flags: review?(dveditz) → review?(benjamin)

Updated

13 years ago
Attachment #184718 - Flags: review?(benjamin)
Attachment #184718 - Flags: review+
Attachment #184718 - Flags: approval-aviary1.1a2?
Attachment #184718 - Flags: approval-aviary1.1a1?

Comment 10

13 years ago
Comment on attachment 184718 [details] [diff] [review]
patch

a=chofmann,  lets get this in quick
Attachment #184718 - Flags: approval-aviary1.1a1? → approval-aviary1.1a1+

Comment 11

13 years ago
Fixed on trunk for 1.1a1.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox1.1
Flags: blocking1.8b2?
Comment on attachment 184718 [details] [diff] [review]
patch

I wish we had constants for the '999' that meant something, or perhaps even a
comment, but a=shaver nonetheless.
Attachment #184718 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
*** Bug 296954 has been marked as a duplicate of this bug. ***
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.