1592502 - initialize all fields of GdkEvents in nsClipboardX11.cpp

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Karl Tomlinson (:karlt)]

+++ This bug was initially created as a clone of Bug #1589092 +++

DispatchSelectionNotifyEvent() doesn't initialize the GdkWindow *requestor field of the GDK_SELECTION_NOTIFY GdkEvent.

gdk_event_copy() would reference the object pointee from selection.requestor
https://gitlab.gnome.org/GNOME/gtk/blob/3.24.11/gdk/gdkevents.c#L735
gdk_event_copy() is used from rewrite_event_for_grabs if the grab window does not match the event window.

I don't know of a situation where that can happen, but it is hard to exclude and bug 1589092 may be evidence that this can happen.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: Bug 1592502 initialize all fields of GdkEvents r?stransky

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/6787b11419c29ce56e6c4d18403bffe27f53788e
https://hg.mozilla.org/mozilla-central/rev/6787b11419c2

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 9105130 [details]
Bug 1592502 initialize all fields of GdkEvents r?stransky

Beta/Release Uplift Approval Request

• User impact if declined: Potential stability/security issues. The relationship between reported crashes and this fix is unverified.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple zero-initialization of currently uninitialized stack variable.
• String changes made/needed: None.

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 9105130 [details]
Bug 1592502 initialize all fields of GdkEvents r?stransky

This can be considered for esr68 when/if landed on beta.

Comment 5

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9105130 [details]
Bug 1592502 initialize all fields of GdkEvents r?stransky

Uplift approved for 71 beta 6, thanks.

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/a3b5cdce3ee7

Comment 7

[Karl Tomlinson (:karlt)]

[Tracking Requested - why for this release]:
Potential stability/security issues. The relationship between reported crashes and this code is unverified.

Comment 8

[Karl Tomlinson (:karlt)]

Comment on attachment 9105130 [details]
Bug 1592502 initialize all fields of GdkEvents r?stransky

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: The relationship between reported crashes and this code is unverified.
  I haven't seen crash reports on release (only Nightly), but the same code is present on 68.
  One option is to wait to see if the patch has an effect on nightly reports, but there is no risk to taking the patch.
• User impact if declined: Potential stability/security issues.
• Fix Landed on Version: 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple zero-initialization of currently uninitialized stack variable.
• String or UUID changes made by this patch: None.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9105130 [details]
Bug 1592502 initialize all fields of GdkEvents r?stransky

Fixes a possible stability issue in GdkEvents. Approved for 68.3esr.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/eda9ff031d24