Chrome JS code injection possible using pluginspage attribute

RESOLVED FIXED

Status

Toolkit Graveyard
Plugin Finder Service
RESOLVED FIXED
12 years ago
3 years ago

People

(Reporter: Doron Rosenberg (IBM), Assigned: Doron Rosenberg (IBM))

Tracking

({fixed-aviary1.0.3})

unspecified
x86
All
fixed-aviary1.0.3
Dependency tree / graph
Bug Flags:
blocking-aviary1.0.3 +

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

12 years ago
Similar to bug 288556 (not fixed by the patch though) :

Code injection into an xul:button's oncommand for manual urls is possible
because a oncommand attribute is set.

Solution: use event listeners.
(Assignee)

Comment 1

12 years ago
Created attachment 179733 [details]
testcase
(Assignee)

Comment 2

12 years ago
Created attachment 179737 [details] [diff] [review]
patch
Attachment #179737 - Flags: superreview?(jst)

Updated

12 years ago
Flags: blocking-aviary1.0.3+
Depends on: 289175
(Assignee)

Comment 3

12 years ago
Steps to reproduce:

- Load testcase
- Click on the missing plugin piece
- It should say no suitable plugins found, and give a manual install button.
- Clicking the manual install button will open a new window and an dialog saying
"Code Injection!".

When fixed, the dialog should now show (new window is fine).
Status: NEW → ASSIGNED
(Assignee)

Comment 4

12 years ago
Correction to above:
  When fixed, the dialog should _NOT_ show (new window is fine).

Updated

12 years ago
Flags: blocking-aviary1.0.3+ → blocking-aviary1.0.3?

Comment 5

12 years ago
Doron:  I can't reproduce this with your testcase using Mozilla/5.0 (Windows; U;
Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050403 Firefox/1.0.3 .  Do I need any
special prefs set to see this problem?
(Assignee)

Comment 6

12 years ago
Created attachment 179746 [details]
new testcase

The patch to 288556 made this a bit harder to exploit after all, new testcase
attached.
Attachment #179733 - Attachment is obsolete: true
Flags: blocking-aviary1.0.3? → blocking-aviary1.0.3+

Updated

12 years ago
Blocks: 289187
(Assignee)

Comment 7

12 years ago
Created attachment 179748 [details] [diff] [review]
fix another theoretical issue

Fixed the theoretical issue of injection via the link to update for missing
plugins.  In reality, the server chokes on the malformed url though :)
Attachment #179737 - Attachment is obsolete: true
Attachment #179748 - Flags: superreview?(jst)
(Assignee)

Updated

12 years ago
Attachment #179737 - Flags: superreview?(jst)
(Assignee)

Comment 8

12 years ago
Comment on attachment 179748 [details] [diff] [review]
fix another theoretical issue

sorry, missed a file.
Attachment #179748 - Attachment is obsolete: true
Attachment #179748 - Flags: superreview?(jst)
Comment on attachment 179737 [details] [diff] [review]
patch

sr=jst
Attachment #179737 - Flags: superreview+
Attachment #179737 - Flags: review?(dbaron)
Comment on attachment 179737 [details] [diff] [review]
patch

r=dbaron too, although worth noting that this currently leaks (bug 241518).  We
can live with that, though.
Attachment #179737 - Flags: review?(dbaron) → review+
(Assignee)

Comment 11

12 years ago
Created attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute
Attachment #179756 - Flags: superreview?(jst)
(Assignee)

Comment 12

12 years ago
I could remove the listeners onunload if needed.
Comment on attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute

sr=jst
Attachment #179756 - Flags: superreview?(jst) → superreview+
Attachment #179756 - Flags: review+
You (In reply to comment #12)
> I could remove the listeners onunload if needed.

You actually can't, since you don't have the listener object anymore.  But don't
worry about it -- we have this leak all over, and plugin install is basically a
one-time thing.
(Assignee)

Updated

12 years ago
Keywords: fixed-aviary1.0.3
Fix released
Group: security
Comment on attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute

Please don't forget to land this on the trunk.
Attachment #179756 - Flags: approval-aviary1.1a+
Attachment #179756 - Flags: approval-aviary1.0.3+
(Assignee)

Comment 17

12 years ago
fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Product: Firefox → Toolkit
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.