Crash [@ mozilla::PrintfTarget::cvt_f ] | MOZ_RELEASE_ASSERT(len <= sizeof(fout))

RESOLVED FIXED in Firefox 65

Status

()

defect
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: bc, Assigned: tromey)

Tracking

(Blocks 1 bug, {assertion, crash})

65 Branch
mozilla66
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox64 wontfix, firefox65 fixed, firefox66 fixed)

Details

(crash signature, )

Attachments

(1 attachment)

Hi Tom, is this something at your wheel house? Thanks!

Flags: needinfo?(ttromey)
Assignee

Comment 2

5 months ago

My current theory is that there is some JS code doing something like
console.log("%500.500f", 1.0) -- that is, specifying a precision and/or
width that will overflow the fixed buffer here.

If so, this is a very old bug :) ... but I think never caught before because,
prior to bug 1060419, the result of SprintfLiteral was not checked.

I'm testing this theory now.

Assignee

Comment 3

5 months ago

Actually I think this is a regression introduced by the switch from PR_vsxprintf
to the mozglue printf :-(

Assignee

Comment 4

4 months ago
mozilla::PrintfTarget::cvt_f release asserts that the desired printf
fit into a statically-sized buffer.  However, this may not be the case
if the user requested a larger width or precision.  Handle this
unusual case by allocating a temporary buffer.

MozReview-Commit-ID: 2WicecHDzDR
Assignee

Updated

4 months ago
Assignee: nobody → ttromey

Comment 6

4 months ago
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/be5403f2da75
do not assert on over-long float printf; r=froydnj

Comment 7

4 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Assignee

Comment 8

4 months ago

Comment on attachment 9035161 [details]
Bug 1517433 - do not assert on over-long float printf; r?froydnj

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: A crasher, due to a regression introduced in bug 1060419.

User impact if declined: Crash if the page calls console.log with certain parameters.

Fix Landed on Version: 66

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It seems low risk to me because it is a reasonably localized patch that lifts an arbitrary limit in some cases.

String or UUID changes made by this patch: None

Attachment #9035161 - Flags: approval-mozilla-esr60?

Given the volume, I'm not convinced we really need this on ESR60. That said, did you want to nominate this for Beta uplift so the fix can ship in 65 at least?

Blocks: 1060419
Flags: needinfo?(ttromey)
Assignee

Comment 10

4 months ago

That said, did you want to nominate this for Beta uplift so the fix can ship in 65 at least?

Sure, will do. Thanks for taking a look.

Flags: needinfo?(ttromey)
Assignee

Comment 11

4 months ago

Comment on attachment 9035161 [details]
Bug 1517433 - do not assert on over-long float printf; r?froydnj

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1060419

User impact if declined: Crash if the page uses certain forms of console.log.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

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): The patch is localized and just lifts an existing static limit in an unusual case.

String changes made/needed: No.

Attachment #9035161 - Flags: approval-mozilla-esr60? → approval-mozilla-beta?

Comment on attachment 9035161 [details]
Bug 1517433 - do not assert on over-long float printf; r?froydnj

[Triage Comment]
Fixes crashes for sites using certain forms of console.log. Approved for 65.0b11.

Attachment #9035161 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.