Closed Bug 1519337 Opened 1 year ago Closed 1 year ago

UTF8CharEnumerator: Off-by-one reads past the end of buffer when buffer ends after bytes that form a valid prefix for a 3-byte or 4-byte UTF-8 sequence

Categories

(Core :: Internationalization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: hsivonen, Assigned: hsivonen)

References

Details

(Keywords: csectype-bounds, regression, sec-other, Whiteboard: [post-critsmash-triage][adv-main66+])

Attachments

(1 file)

Some bound checks in my rewrite of UTF8CharEnumerator are bogus. By reading the code, it appears that it's possible to read one byte past the end of the input.

(Fortunately, usage patterns suggest low actual security impact.)

Blocks: 1402247
Group: core-security → dom-core-security
Keywords: regression

Given the severity, it sounds like this can just ride the trains whenever it's ready to land.

Thanks for the r+.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #2)

Given the severity, it sounds like this can just ride the trains whenever it's ready to land.

Landing queued. Furthermore, what the patch does is so obvious that it didn't make sense to try to obscure the test case or the commit message.

For clarity about the severity:

Previously, the code returned correct-appearing results and, therefore, passed the previous tests that tried to test buffer end boundary conditions, because when the end pointer points to a zero terminator, the out-of-bounds read reads an allocated byte and the byte doesn't fit the UTF-8 sequence.

If there isn't a zero terminator, if the memory access doesn't segfault, there is disclosure of at most 7 bits, but in our usage, the bits are disclosed to our internal callers that don't make it practical to disclose the bits further out.

I filed this as a security bug just in case, but having inspected the callers this doesn't even meet the criteria for "low", i.e. the severity is lower than low, AFAICT. I suggest unhiding.

Keywords: sec-other
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main66+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.