Closed Bug 1412445 Opened 2 years ago Closed 2 years ago

gMainPane.QueryInterface doesn't properly check its aIID argument

Categories

(Firefox :: Preferences, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox58 --- fixed

People

(Reporter: myk, Assigned: myk)

Details

Attachments

(1 file)

gMainPane.QueryInterface doesn't properly check its aIID argument because of mis-encapsulation of its conditionals:

  QueryInterface(aIID) {
    if (aIID.equals(Ci.nsIObserver) ||
      aIID.equals(Ci.nsIDOMEventListener ||
        aIID.equals(Ci.nsISupports)))
      return this;

    throw Cr.NS_ERROR_NO_INTERFACE;
  },

The refactoring in bug 1349689 <https://hg.mozilla.org/mozilla-central/rev/9db0cec976bb>, which changed the indentation of the lines, made this more obvious; but it didn't introduce the problem.

It looks like I introduced it 10 years ago in bug 377784 <https://hg.mozilla.org/mozilla-central/rev/045452b30d72da2f45f0a89ce9247227b39dc38c>.

In any case, the fix is straightforward and can be made even safer with XPCOMUtils.generateQI.  (I wonder if we should do a more general search/replace for QueryInterface implementations that could use XPCOMUtils.generateQI.)
Attachment #8923020 - Flags: review?(rchien)
Comment on attachment 8923020 [details] [diff] [review]
replace custom QI impl with XPCOMUtils.generateQI call

Review of attachment 8923020 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. thanks!
Attachment #8923020 - Flags: review?(rchien) → review+
Pushed by myk@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/726e06daf2d7
replace custom QI impl with XPCOMUtils.generateQI call; r=rickychien
https://hg.mozilla.org/mozilla-central/rev/726e06daf2d7
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.