1772171 - Move ToDouble back to nsTString

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nika Layzell [:nika] (ni? for response)]

In bug 1768418, we move ToDouble to nsTStringRepr, however I missed in review that it uses PR_strtod which doesn't take a string length and could overrun when used on a non-null-terminated string.

We should move the method back to nsTString to avoid this issue for now. I have some local changes which could allow it to be implemented on nsTStringRepr again, but they're significant enough that we probably can't safely uplift.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

This is probably not an active security issue, as there appear to be no new callers introduced since bug 1768418 which call the method on a substring (the build completed successfully with no changes to code outside of nsstring after moving it back). We should probably uplift the fix though to protect future uplifts.

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1772171 - Move ToDouble back to nsTString, r=#xpcom-reviewers

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 9279143 [details]
Bug 1772171 - Move ToDouble back to nsTString, r=#xpcom-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Future uplifts may introduce a sec issue if they call ToDouble on a non-nsCString string.
  There should be no impact on existing code.
• Is this code covered by automated tests?: No
• 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): Small change to revert an API change. Should have no actual behaviour change and is just for guarding against regressions caused by future uplifts.
• String changes made/needed: None
• Is Android affected?: Yes

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Move ToDouble back to nsTString, r=xpcom-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/0ca2d284919ed425bedc5ccd760e6d635e470a7f
https://hg.mozilla.org/mozilla-central/rev/0ca2d284919e

Comment 5

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9279143 [details]
Bug 1772171 - Move ToDouble back to nsTString, r=#xpcom-reviewers

Approved for 102 beta 5, thanks.

Comment 6

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/bd96d7d79065