Last Comment Bug 289171 - Chrome JS code injection possible using pluginspage attribute
: Chrome JS code injection possible using pluginspage attribute
Status: RESOLVED FIXED
: fixed-aviary1.0.3
Product: Toolkit Graveyard
Classification: Graveyard
Component: Plugin Finder Service (show other bugs)
: unspecified
: x86 All
: -- normal (vote)
: ---
Assigned To: Doron Rosenberg (IBM)
:
:
Mentors:
Depends on: 289175
Blocks: 289187
  Show dependency treegraph
 
Reported: 2005-04-05 11:53 PDT by Doron Rosenberg (IBM)
Modified: 2014-09-24 05:45 PDT (History)
8 users (show)
dbaron: blocking‑aviary1.0.3+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
testcase (242 bytes, text/html)
2005-04-05 11:54 PDT, Doron Rosenberg (IBM)
no flags Details
patch (1.32 KB, patch)
2005-04-05 11:58 PDT, Doron Rosenberg (IBM)
dbaron: review+
jst: superreview+
Details | Diff | Splinter Review
new testcase (259 bytes, text/html)
2005-04-05 13:10 PDT, Doron Rosenberg (IBM)
no flags Details
fix another theoretical issue (1.81 KB, patch)
2005-04-05 13:35 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
fix theoretical issue and remove onclick attribute (2.75 KB, patch)
2005-04-05 13:57 PDT, Doron Rosenberg (IBM)
dbaron: review+
jst: superreview+
dbaron: approval‑aviary1.0.3+
dbaron: approval‑aviary1.1a1+
Details | Diff | Splinter Review

Description Doron Rosenberg (IBM) 2005-04-05 11:53:39 PDT
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.
Comment 1 Doron Rosenberg (IBM) 2005-04-05 11:54:23 PDT
Created attachment 179733 [details]
testcase
Comment 2 Doron Rosenberg (IBM) 2005-04-05 11:58:57 PDT
Created attachment 179737 [details] [diff] [review]
patch
Comment 3 Doron Rosenberg (IBM) 2005-04-05 12:33:38 PDT
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).
Comment 4 Doron Rosenberg (IBM) 2005-04-05 12:34:43 PDT
Correction to above:
  When fixed, the dialog should _NOT_ show (new window is fine).
Comment 5 Jay Patel [:jay] 2005-04-05 12:55:30 PDT
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?
Comment 6 Doron Rosenberg (IBM) 2005-04-05 13:10:49 PDT
Created attachment 179746 [details]
new testcase

The patch to 288556 made this a bit harder to exploit after all, new testcase
attached.
Comment 7 Doron Rosenberg (IBM) 2005-04-05 13:35:55 PDT
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 :)
Comment 8 Doron Rosenberg (IBM) 2005-04-05 13:39:32 PDT
Comment on attachment 179748 [details] [diff] [review]
fix another theoretical issue

sorry, missed a file.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2005-04-05 13:52:36 PDT
Comment on attachment 179737 [details] [diff] [review]
patch

sr=jst
Comment 10 David Baron :dbaron: ⌚️UTC-10 2005-04-05 13:56:11 PDT
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.
Comment 11 Doron Rosenberg (IBM) 2005-04-05 13:57:58 PDT
Created attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute
Comment 12 Doron Rosenberg (IBM) 2005-04-05 13:59:46 PDT
I could remove the listeners onunload if needed.
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2005-04-05 14:30:57 PDT
Comment on attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute

sr=jst
Comment 14 David Baron :dbaron: ⌚️UTC-10 2005-04-05 14:46:14 PDT
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.
Comment 15 Daniel Veditz [:dveditz] 2005-04-15 19:52:16 PDT
Fix released
Comment 16 David Baron :dbaron: ⌚️UTC-10 2005-04-15 21:48:23 PDT
Comment on attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute

Please don't forget to land this on the trunk.
Comment 17 Doron Rosenberg (IBM) 2005-04-19 08:09:56 PDT
fixed on trunk.

Note You need to log in before you can comment on or make changes to this bug.