Closed Bug 658657 Opened 15 years ago Closed 15 years ago

Validator should check for duplicate file entries on packages and show error if found

Categories

(addons.mozilla.org Graveyard :: Developer Pages, defect, P3)

Product:
addons.mozilla.org Graveyard
Component:
Developer Pages
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Q3 2011

People

(Reporter: benjamin, Assigned: basta)

References

(
URL
)

Details

(Whiteboard: [ReviewTeam])

The forecastfox XPI at https://addons.mozilla.org/firefox/downloads/latest/398/addon-398-latest.xpi?src=addondetail has two copies of the root chrome.manifest file. This is probably accidental, and they are probably the same, but we should check it out and try to prevent that from happening in the future. I'm not sure which of the two Firefox might use, it probably depends on the JAR implementation.
After a little poking at the files, they are the same, so there is no actual urgent problem here.
Assignee: nobody → jorge
I only see one copy on the source viewer and also if I download the file and decompress it. What do you mean there are 2 copies? Are there 2 entries in the ZIP manifest or something like that?
What source viewer? If you type `unzip forecastfox.xpi` you get: Archive: forecastfox.xpi Length Date Time Name --------- ---------- ----- ---- 1233923 09-20-2010 11:16 chrome/forecastfox.jar 1605 09-20-2010 11:16 install.rdf 528 09-20-2010 11:16 chrome.manifest 528 09-20-2010 11:16 chrome.manifest 1782 08-26-2010 23:40 icon.png --------- ------- 1238366 5 files
unzip -l, of course
There are two different copies of the same file in the zip. It's probably caused by some bad zip tool that the author is using. Since it's the same file it's basically safe, if it were different files I'm not sure which of the two we'd end up extracting and using.
Is it possible to include 2 different files with the same name in a zip file? Could that be used to bypass any of our checks?
The zip format allows it, most zip tools shouldn't let you (adding a file of the same name to the zip file should just replace the existing entry). If Firefox uses one of the files and your validation tools use the other one (it'll depend on whatever library is doing extraction which one gets used I think) then yes I guess conceivably it could allow some files to bypass review
Matt, can you look into this? I would like the validator to show an error if the XPI (or internal JAR) has any duplicate entries, given that it could potentially cause bad code to go past our other checks.
Assignee: jorge → nobody
Severity: normal → minor
Component: Add-on Security → Developer Pages
OS: Windows 7 → All
Priority: -- → P3
QA Contact: security → developers
Hardware: x86 → All
Summary: Forecastfox XPI has two copies of chrome.manifest (malformed XPI) → Validator should check for duplicate file entries on packages and show error if found
Whiteboard: [required amo-editors]
Target Milestone: --- → Q2 2011
-> basta
Assignee: nobody → mbasta
I'm going to have to look into whether the Python zipfile library exposes multiple copies of the same file, since you access them by name. If the Mozilla and Python implementations work differently, then there definitely is a vulnerability.
It can: >>> import zipfile >>> z = zipfile.ZipFile('forecastfox.xpi') >>> z.namelist() ['chrome/forecastfox.jar', 'install.rdf', 'chrome.manifest', 'chrome.manifest', 'icon.png'] >>>
Target Milestone: Q2 2011 → Q3 2011
Merged: https://github.com/mozilla/amo-validator/commit/3a369ba19d448742cb4bf00b04467e41b50dd930
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...
Whiteboard: [required amo-editors] → [ReviewTeam]
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.