JS::StringIsUTF8, used for Wasm name validation, must be more stringent

RESOLVED FIXED in Firefox 57

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: lth, Assigned: lth)

Tracking

unspecified
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 months ago
As pointed out in bug 1396220, the checker is too lenient with illegal pairs and some out-of-range values.  I failed to notice those restrictions when I implemented the code earlier.  The correct restrictions are in https://github.com/WebAssembly/spec/blob/master/interpreter/binary/utf8.ml.

We'll not wait for bug 1396220 to be resolved (since that involves politics), but will fix this in our codebase and let that thing play out independently.
(Assignee)

Comment 1

6 months ago
Created attachment 8904224 [details] [diff] [review]
bug1396557-stricter-utf8.patch

This adds the value computation and the range checks from the reference implementation, see link in comment 0.
Attachment #8904224 - Flags: review?(luke)
(In reply to Lars T Hansen [:lth] from comment #1)
> This adds the value computation

FWIW, Computing the scalar value isn't logically necessary. The validity can be decided from the byte values alone by limiting the range of the first continuation depending on what the value of the lead was.
(Assignee)

Comment 3

6 months ago
(In reply to Henri Sivonen (:hsivonen) from comment #2)
> (In reply to Lars T Hansen [:lth] from comment #1)
> > This adds the value computation
> 
> FWIW, Computing the scalar value isn't logically necessary. The validity can
> be decided from the byte values alone by limiting the range of the first
> continuation depending on what the value of the lead was.

OK, that's an interesting fact.  Even so I'll leave that for future work, I think.

Updated

6 months ago
Attachment #8904224 - Flags: review?(luke) → review+
(Assignee)

Updated

6 months ago
Keywords: checkin-needed

Comment 4

6 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8822dc7f2d3c
Make JS::StringIsUTF8 stricter. r=luke
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8822dc7f2d3c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.