Closed Bug 1739219 Opened 3 years ago Closed 3 years ago

SetLength() and GetMutableData() are dangerous APIs

Categories

(Core :: XPCOM, enhancement)

Product:
Core
Component:
XPCOM
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
97 Branch
Tracking Flags:
Tracking Status
firefox-esr91 97+ fixed
firefox95 --- wontfix
firefox96 --- wontfix
firefox97 + fixed

People

(Reporter: mccr8, Assigned: nika)

References

Details

(Keywords: sec-want, Whiteboard: [post-critsmash-triage][adv-main97-][adv-esr91.6-])

As seen in bug 1738237, nsTSubstring<T>::SetLength() takes an nsTSubString::size_type argument. Unfortunately this means that if you pass in a 64-bit value it will be implicitly coerced into a 32-bit value, which can overflow. It would be nice if we were more resistant to this kind of issue.

One approach would be to have a (non-nsTSubString) size_t overload that does the CheckedInt conversion. Or maybe we need bounds checking on whatever does a read or write after the SetLength()?

Summary: SetLength() is dangerous API → SetLength() is a dangerous API
See Also: → CVE-2021-43537
Depends on: 1741201
Depends on: 1741210

Decoder mentioned GetMutableData() as another thing that deals with lengths.

Summary: SetLength() is a dangerous API → SetLength() and GetMutableData() are dangerous APIs

Nika is working on some kind of big fix here, so I'll assign this to her for now.

Assignee: nobody → nika
Depends on: 1741665
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Group: dom-core-security → core-security-release
status-firefox95: --- → wontfix
status-firefox96: --- → affected
status-firefox97: --- → fixed
status-firefox-esr91: --- → affected
Target Milestone: --- → 97 Branch
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main97-]
Whiteboard: [post-critsmash-triage][adv-main97-] → [post-critsmash-triage][adv-main97-][adv-esr91.6-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.