Doron Rosenberg (IBM) Assignee
	Description • 20 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.

	Doron Rosenberg (IBM) Assignee
	Comment 3 • 20 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).

	Doron Rosenberg (IBM) Assignee
	Comment 4 • 20 years ago

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

	Jay Patel [:jay]
	Comment 5 • 20 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?

	Doron Rosenberg (IBM) Assignee
	Comment 6 • 20 years ago

The patch to 288556 made this a bit harder to exploit after all, new testcase
attached.

	Doron Rosenberg (IBM) Assignee
	Comment 7 • 20 years ago

Fixed the theoretical issue of injection via the link to update for missing
plugins.  In reality, the server chokes on the malformed url though :)

	Doron Rosenberg (IBM) Assignee
	Comment 8 • 20 years ago

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

sorry, missed a file.

	Johnny Stenback (:jst)
	Comment 9 • 20 years ago

Comment on attachment 179737 [details] [diff] [review]
patch

sr=jst

	David Baron :dbaron:
	Comment 10 • 20 years ago

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.

	Doron Rosenberg (IBM) Assignee
	Comment 12 • 20 years ago

I could remove the listeners onunload if needed.

	Johnny Stenback (:jst)
	Comment 13 • 20 years ago

Comment on attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute

sr=jst

	David Baron :dbaron:
	Comment 14 • 20 years ago

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.

	Daniel Veditz [:dveditz]
	Comment 15 • 20 years ago

Fix released

	David Baron :dbaron:
	Comment 16 • 20 years ago

Comment on attachment 179756 [details] [diff] [review]
fix theoretical issue and remove onclick attribute

Please don't forget to land this on the trunk.

	Doron Rosenberg (IBM) Assignee
	Comment 17 • 20 years ago

fixed on trunk.