Closed Bug 257523 Opened 15 years ago Closed 15 years ago

Text fields give scripts access to the user's clipboard

Categories

(Core :: DOM: Events, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: gaubugzilla, Assigned: gaubugzilla)

Details

(Keywords: fixed-aviary1.0, fixed1.7, regression, Whiteboard: [sg:fix] fixed1.7.3)

Attachments

(5 files)

Text fields accept script-generated events. You cannot send Ctrl-C - this is a
menu shortcut, so untrusted events will be rejected. But you can send Ctrl-Ins,
which is implemented in the text field itself. This way you can put every text
you want into the user's clipboard. And you can send Shift-Ins to insert
whatever you have in your clipboard into the text field.

I've tested this on Windows but it should work as well at least with KDE, it has
same shortcuts.
These shortcuts are defined in
content/xbl/builtin/(win|unix)/platformHTMLBindings.xml. XBL only rejects
non-trusted events for chrome content but these text fields are not chrome...
Assignee: nobody → jst
Component: Layout: Form Controls → DOM: Events
Whiteboard: [sg:fix]
Severity critical, because I keep copy&pasting passwords, assume others do as well.
Reproduced both read and write on Mozilla 1.7.x branch on Linux. Only works when
I copied using the explicit "CLIPBOARD" (via menu, shortcut etc.), not the
"PRIMARY" clipboard (just select).
<http://freedesktop.org/Standards/clipboards-spec/clipboards.txt>
Severity: major → critical
Flags: blocking1.8a4?
Flags: blocking1.7.x?
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0?
OS: Windows XP → All
Hardware: PC → All
Attached patch Proposed patchSplinter Review
This prevents commands from being triggered by non-trusted events, even if the
widget itself isn't trusted.
Attachment #157608 - Flags: review?(jst)
Comment on attachment 157608 [details] [diff] [review]
Proposed patch

Seems reasonable to me. r=jst
Attachment #157608 - Flags: review?(jst) → review+
Comment on attachment 157608 [details] [diff] [review]
Proposed patch

>   // See if our event receiver is a content node (and not us).
>   PRBool isXULKey = (mType & NS_HANDLER_TYPE_XUL);
> 
>-  //XUL handlers shouldn't be triggered by non-trusted events.
>-  if (isXULKey) {
>+  //XUL handlers and commands shouldn't be triggered by non-trusted events.
>+  if (isXULKey || (mType & NS_HANDLER_TYPE_XBL_COMMAND)) {

Since there's another (mType & NS_HANDLER_TYPE_XBL_COMMAND) a few lines down
you could define isXBLCommand like isXULKey above for readability. Or not --
your call.

sr=dveditz
Attachment #157608 - Flags: superreview+
Attachment #157608 - Flags: approval1.7.x?
Attachment #157608 - Flags: approval-aviary?
Is there anything that could be legitimately relying on the old behavior? I was
thinking of some sort of QA test harness poking in events, but I guess a chrome
app could create trusted events for that.
Comment on attachment 157608 [details] [diff] [review]
Proposed patch

a=chofmann for the branches
Attachment #157608 - Flags: approval1.7.x?
Attachment #157608 - Flags: approval1.7.x+
Attachment #157608 - Flags: approval-aviary?
Attachment #157608 - Flags: approval-aviary+
Flags: blocking1.7.x?
Flags: blocking1.7.x+
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0PR+
we run as chrome and i actually have keyhandling off. supporting midas is going
to be so much fun.
Fixed on aviary and 1.7 branches, and on the trunk. Marking FIXED.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Fix checked into the 1.7.2 branch also
Whiteboard: [sg:fix] → [sg:fix] fixed1.7.2+
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0?
Verified with the two testcases on Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.7.3) Gecko/20040910
Group: security
Whiteboard: [sg:fix] fixed1.7.2+ → [sg:fix] fixed1.7.3
Flags: blocking1.8a4?
The test cases still work (i.e. the bug still exists) for me in Linux with the
GTK2/XFT nightly builds of both Mozilla and Firefox; however, it does seems
fixed with the GTK1 builds.
Reopening, as this is still not fixed on linux.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Just a guess - is this because of bug 257405?
Looking at the patch - this seems to be the case. DOMEventToNativeKeyEvent()
should return PR_FALSE for non-trusted events.
This fixes the regression caused by bug 257405. AFAICT this is the only place
where we need a check.
Attachment #160498 - Flags: superreview?(dveditz)
Attachment #160498 - Flags: review?(jst)
Keywords: regression
Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

Thanks for the quick fix, Wladimir!

r=jst
Attachment #160498 - Flags: review?(jst) → review+
Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

sr=dveditz

bug 257523 landed on the branches, so we'll want this regression fix there as
well.
Attachment #160498 - Flags: superreview?(dveditz)
Attachment #160498 - Flags: superreview+
Attachment #160498 - Flags: approval1.7.x?
Attachment #160498 - Flags: approval-aviary?
Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

a=mkaply for both
Attachment #160498 - Flags: approval1.7.x?
Attachment #160498 - Flags: approval1.7.x+
Attachment #160498 - Flags: approval-aviary?
Attachment #160498 - Flags: approval-aviary+
Fixed on trunk, aviary and 1.7 branches. Thanks for the patch, Wladimir!
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
verified with windows 1.7.5 12/15
Status: RESOLVED → VERIFIED
verified with Mac 1.75 2004-12-16-09
Assignee: jst → trev
I only see Ctrl-Ins and Shift-Ins mentioned.

Are we taking care of "Shift-Del" which is equivalent to ctrl-x

Also if we can get a test case, it will be great help.
The fix here is general and takes care of all keys handled by XBL bindings.
You need to log in before you can comment on or make changes to this bug.