Closed Bug 1592502 Opened 8 months ago Closed 8 months ago

initialize all fields of GdkEvents in nsClipboardX11.cpp

Categories

(Core :: Widget: Gtk, defect)

Desktop
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 71+ fixed
firefox70 --- wontfix
firefox71 + fixed
firefox72 + fixed

People

(Reporter: karlt, Assigned: karlt)

References

Details

(Keywords: crash, csectype-wildptr, sec-moderate, Whiteboard: [adv-main71+r][adv-esr68.3+r])

Crash Data

Attachments

(1 file)

+++ 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.

Group: core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

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.
Attachment #9105130 - Flags: approval-mozilla-esr68?
Attachment #9105130 - Flags: approval-mozilla-beta?

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.

Attachment #9105130 - Flags: approval-mozilla-esr68?

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

Uplift approved for 71 beta 6, thanks.

Attachment #9105130 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Crash Signature: nsRetrievalContextX11::WaitForX11Content | nsRetrievalContextX11::GetTargets] [@ raise | abort | __libc_message | __fortify_fail_abort | __stack_chk_fail | nsRetrievalContextX11::WaitForX11Content | nsTArray_Impl<T>::AppendElements<T> | _fini] [@ raise… → nsRetrievalContextX11::WaitForX11Content | nsRetrievalContextX11::GetTargets] [@ raise | abort | __libc_message | __fortify_fail_abort | __stack_chk_fail | nsRetrievalContextX11::WaitForX11Content | nsTArray_Impl<T>::AppendElements<T> | _fini]

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.
Attachment #9105130 - Flags: approval-mozilla-esr68?

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.

Attachment #9105130 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Whiteboard: [adv-main71+r] → [adv-main71+r][adv-esr68.3+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.