Closed Bug 635087 Opened 10 years ago Closed 10 years ago

nsCryptoRunnable::Run leaks JSPrincipals

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mwu, Assigned: mwu)

References

Details

(Keywords: memory-leak)

Attachments

(1 file, 1 obsolete file)

Attached patch Fix (obsolete) — Splinter Review
No description provided.
Attachment #513298 - Flags: review?(kaie)
Attachment #513298 - Flags: review?(kaie) → review?(jst)
Comment on attachment 513298 [details] [diff] [review]
Fix

Good catch! r=jst, but while you're here, want to add:

  JSAutoEnterCompartment ac;

  if (!ac.enter(cx, realObj)) {
    return NS_ERROR_FAILURE;
  }

right below the JSAutoRequest, and move both lines up top so that doesn't also leak principals? That way we'll ensure that the JS_EvaluateScriptForPrincipals() call always runs in the right compartment.
Attachment #513298 - Flags: review?(jst) → review+
Attached patch Fix, v2Splinter Review
Like this? Unfortunately don't know enough about the js context stack to know if this is ok.
Attachment #513298 - Attachment is obsolete: true
Attachment #520778 - Flags: review?(jst)
Comment on attachment 520778 [details] [diff] [review]
Fix, v2

Yup, that looks right to me. Thanks! r=jst
Attachment #520778 - Flags: review?(jst) → review+
http://hg.mozilla.org/mozilla-central/rev/53ab33f0fd8c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Blocks: mlk-fx5+
Can someone confirm if this is fixed?
You need to log in before you can comment on or make changes to this bug.