346443 - Signing hole -- META-INF files not checked, can be installed

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Daniel Veditz [:dveditz]]

XPInstall signature verification explicitly skips the META-INF/ directory because that's where the signature files themselves are. This provides a potential hole through which a malicious install could get unsigned or modified content into the install.

A legitimate install is no problem: the install itself won't touch any of the files in META-INF/ so an attacker adding files there simply bloats the envelope but they won't end up on disk.

To exploit this hole the install script itself must cooperate by installing a file from META-INF/, which can then be altered later and will not get signed. If we're positing a malicious install then it doesn't need to bother with this hole, the signed contents could be malicious since the user has trusted the signer.

We knew this when we designed this system and considered that it was not a practical problem. I'm filing this bug to document this decision, and also to investigate whether the newer Extension Manager mechanism runs afoul of this design.

I think the Extension Manager simply unpacks the entire extension, which might result in extra META-INF/ files landing on disk. Seems like they'd be fairly inert, but there may be some way to get at them. And you could contaminate a signed install on a hacked mirror with child porn or something in an attempt to get someone in trouble.

XPInstall addFile and addDirectory should fail on any reference to the META-INF directory.

The Extension Manager should not unpack extra files (though it's nice to preserve the signatures, maybe we could figure out how to do both).

Comment 1

[Daniel Veditz [:dveditz]]

calling this sg:low because the install script *is* signed: if an installer did this it was with the cooperation of the signer.

Comment 2

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

Daniel, since we don't use xpinstall anymore is this bug still valid?

Comment 3

[Dave Townsend [:mossop]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #2)
> Daniel, since we don't use xpinstall anymore is this bug still valid?

I think this is still a problem, though now in the add-ons manager boat. It would require an add-ons files to explicitly reference stuff under META-INF (e.g. add that dir to chrome.manifest) for it to be exploitable. It's also now problematic since with mostly install the XPI itself. We'd have to strip out files from META-INF from that XPI in order to stop this being a problem.

Comment 4

[Daniel Veditz [:dveditz]]

This was fixed for add-on signing when we switched to use AppSignatureVerification.cpp instead of the old JAR-signing code. Only the three expected meta files are excluded and anything else in that META-INF directory is checked.
https://dxr.mozilla.org/mozilla-central/rev/5069348353f8fc1121e632e3208da33900627214/security/apps/AppSignatureVerification.cpp#1339

The old JAR signing code still exists but is not used. By itself it only verified the signed status of individual files, not the integrity of entire archives. That was work the add-on/xpinstall code had to do on top.