1142681 - [WebCrypto] |CreateECParamsForCurve| leaks |params|

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Eric Rahm [:erahm]]

Coverity indicates that |CreateECParamsForCurve| leaks |params| if the sanity check [1] fails.

[1] https://hg.mozilla.org/mozilla-central/annotate/0190a1d17294/dom/crypto/WebCryptoCommon.h#l320

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

The SECItem is alloced in the ScopedPLArenaPool given by the caller. Once that goes out of scope we should free the memory as well here. This is assuming that aArena ≠ null, so should we maybe add an assertion here? All existing callers pass a non-null arena but we could avoid future accidental leaks?

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler]]

That sounds reasonable. I don't know if coverity will be satisfied by that, but maybe we could annotate it or something.

Comment 3

[Eric Rahm [:erahm]]

An assertion sounds good. If Coverity doesn't pick up on it someone triaging defects probably will.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1142681-Let-CreateECParamsForCurve-assert-a-non-.patch

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fcf4cac17e3a

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/fcf4cac17e3a

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Eric, out of curiosity: does Coverity still report a leak?

Comment 8

[Eric Rahm [:erahm]]

(In reply to Tim Taubert [:ttaubert] from comment #7)
> Eric, out of curiosity: does Coverity still report a leak?

The latest run shows it as fixed, so a release assertion seems to have done the job.