Page MenuHomeFreeBSD

ee: Bound the number of bytes we'll skip in utf8_prev()
Needs ReviewPublic

Authored by markj on Sat, Jun 27, 7:46 PM.

Details

Reviewers
bapt
Group Reviewers
secteam
Summary

utf8_prev() will walk backwards unless it sees a non-continuation byte,
but this is dangerous in the face of malformed UTF-8 input. In
particular, delete() will copy the skipped-over bytes into a
heap-allocated buffer, d_char, which is fixed at 5 bytes.

Make utf8_prev() refuse to step more than four bytes backward, to avoid
the overflow in delete().

Fixes: 62fba0054d9e ("ee: add unicode support")
Reported by: Sayono Hiragi

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 74336
Build 71219: arc lint + arc unit

Event Timeline

markj requested review of this revision.Sat, Jun 27, 7:46 PM

Thinking a bit more, this is maybe not the right solution, delete() should perhaps handle invalid sequences instead.

des added inline comments.
contrib/ee/ee.c
245–247

de gustibus et coloribus non est disputandum, but I find this a little more readable.

Thinking a bit more, this is maybe not the right solution, delete() should perhaps handle invalid sequences instead.

I was thinking along the same lines, because although utf8_prev() is now safe, it no longer does what it promises.

In D57913#1327630, @des wrote:

Thinking a bit more, this is maybe not the right solution, delete() should perhaps handle invalid sequences instead.

I was thinking along the same lines, because although utf8_prev() is now safe, it no longer does what it promises.

d_char is allocated, so nothing prevents us from reallocating it (keeping track of its size of course) if we need to delete more than five bytes.