Prevent updates changing short_name

RESOLVED FIXED in Firefox 39

Status

defect
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: pauljt, Assigned: tedders1)

Tracking

({dev-doc-needed, feature, uiwanted})

Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(feature-b2g:2.2+, firefox37 wontfix, firefox38 wontfix, firefox39 fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [dependency: marketplace-partners][systemsfe])

Attachments

(1 attachment, 1 obsolete attachment)

AFAIU, bug 1001861 will result in always only showing the short_name to the user when such value is present in the manifest. We currently prevent updates from changing an apps name, to prevent apps spoofing other apps after they have been installed.

If we are going to replace all places where app name is shown with shortname, we should apply this same restriction to short_name.
Blocks: 1001861
No longer blocks: 1075704
No longer depends on: 1001861, 1134680, 1121466, 1124144, 1124146, 1131338
Actually there's a AppUtils funciton that does the actuall manifest checking. I guess just this can be updated:

https://mxr.mozilla.org/mozilla-central/source/dom/apps/AppsUtils.jsm#520
security related
blocking-b2g: --- → 2.2+
Can you take this one?
Flags: needinfo?(tclancy)
Assignee: nobody → tclancy
Flags: needinfo?(tclancy)
blocking-b2g: 2.2+ → ---
feature-b2g: --- → 2.2+
This is another one that got missed as a feature, not a blocking-b2g ...so you're aware bajaj. Risk is low
Flags: needinfo?(bbajaj)
Posted patch bug-1137498-fix.patch (obsolete) — Splinter Review
Attachment #8575267 - Flags: review?(fabrice)
Comment on attachment 8575267 [details] [diff] [review]
bug-1137498-fix.patch

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

r=me with nits fixed.
Thanks for adding tests!

::: dom/apps/AppsUtils.jsm
@@ +530,4 @@
>        for (let locale in aNewManifest.locales) {
> +        let newLocaleEntry = aNewManifest.locales[locale];
> +
> +        let oldLocaleEntry = aOldManifest && 'locales' in aOldManifest &&

nit: double quotes on "locales"

@@ +535,5 @@
> +
> +        if (newLocaleEntry.name) {
> +          // In case previous manifest didn't had a name,
> +          // we use the default app name
> +          newLocaleEntry.name = 

nit: trailing whitespace

@@ +541,3 @@
>          }
> +        if (newLocaleEntry.short_name) {
> +          newLocaleEntry.short_name = 

here too
Attachment #8575267 - Flags: review?(fabrice) → review+
Attachment #8575267 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/82536efb0b2e
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [dependency: marketplace-partners][systemsfe] → [dependency: marketplace-partners][systemsfe][fixed-in-fx-team]
Flags: needinfo?(bbajaj)
https://hg.mozilla.org/mozilla-central/rev/82536efb0b2e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [dependency: marketplace-partners][systemsfe][fixed-in-fx-team] → [dependency: marketplace-partners][systemsfe]
Target Milestone: --- → mozilla39
Comment on attachment 8576683 [details] [diff] [review]
bug-1137498-fix.patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
Bug 1001861

User impact if declined: 
Possible security risk.

Testing completed: 
Tests are included in the patch.

Risk to taking this patch (and alternatives if risky): 
None forseen.

String or UUID changes made by this patch:
None
Attachment #8576683 - Flags: approval-mozilla-b2g37?
Attachment #8576683 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.