Paul Theriault [:pauljt] (no longer reading bugmail) Reporter
	Description • 6 years ago

mar_sign.c has a double free bug. extractedSignature is freed on line 536[1]. If we then hit an error [2] we goto failure case which also frees extractedSignature on line 576[3].

It's only mar_sign, so using this on untrusted input would seem unlikely, but filing since we should probably fix it anyways.  


[1] https://searchfox.org/mozilla-central/source/modules/libmar/sign/mar_sign.c#536 
[2] https://searchfox.org/mozilla-central/source/modules/libmar/sign/mar_sign.c#541
[3] https://searchfox.org/mozilla-central/source/modules/libmar/sign/mar_sign.c#576

	Alex Gaynor [:Alex_Gaynor]
	Comment 1 • 6 years ago

The only place this code is reachable from is the MAR CLI tool: https://searchfox.org/mozilla-central/source/modules/libmar/tool/mar.c#309

	June Wilde (she/her) [:jewilde] Assignee
	Comment 3 • 6 years ago

I added line 537 to mar_sign.c to set extractedSignature to null before any of the error checks, that way if a failure occurs before extractedSignature is set again on line 553 the second call to free on line 578 will essentially be free(NULL);

	Daniel Veditz [:dveditz]
	Comment 4 • 6 years ago

"found in an audit" is not sec-audit. This is either sec-low if we think we can reach it (comment 1) or sec-other (or unhidden entirely) if we think we can't.

	Robert Strong (they/them - no direct email)
	Comment 5 • 6 years ago

(In reply to Daniel Veditz [:dveditz] from comment #4)
> "found in an audit" is not sec-audit. This is either sec-low if we think we
> can reach it (comment 1) or sec-other (or unhidden entirely) if we think we
> can't.
This only affects a build tool

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 6 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/379d79841c5d439654d77f264f7c9d730863059e

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 7 • 6 years ago

https://hg.mozilla.org/mozilla-central/rev/379d79841c5d