AK: Fix buffer overrun in Utf8CodepointIterator::operator++

The old implementation tried to move forward as long as the current
byte looks like a UTF-8 character continuation byte (has its two
most significant bits set to 10). This is correct as long as we assume
the string is actually valid UTF-8, which we do (we also have a separate
method that can check whether it is the case).

We can't, however, assume that the data after the end of our string
is also valid UTF-8 (in fact, we're not even allowed to look at data
outside out string, but it happens to a valid memory region most of
the time). If the byte after the end of our string also has its most
significant bits set to 10, we would move one byte forward, and then
fail the m_length > 0 assertion.

One way to fix this would be to add a length check inside the loop
condition. The other one, implemented in this commit, is to reimplement
the whole function in terms of decode_first_byte(), which gives us
the length as encoded in the first byte. This also brings it more
in line with the other functions around it that do UTF-8 decoding.
This commit is contained in:
Sergey Bugaev 2019-09-08 18:24:54 +03:00 committed by Andreas Kling
parent e386579436
commit 1fb6a7d893
Notes: sideshowbarker 2024-07-19 12:11:09 +09:00

View file

@ -123,11 +123,18 @@ bool Utf8CodepointIterator::operator!=(const Utf8CodepointIterator& other) const
Utf8CodepointIterator& Utf8CodepointIterator::operator++()
{
do {
ASSERT(m_length > 0);
m_length--;
m_ptr++;
} while (m_ptr[0] >> 6 == 2);
ASSERT(m_length > 0);
int codepoint_length_in_bytes;
u32 value;
bool first_byte_makes_sense = decode_first_byte(*m_ptr, codepoint_length_in_bytes, value);
ASSERT(first_byte_makes_sense);
(void)value;
ASSERT(codepoint_length_in_bytes <= m_length);
m_ptr += codepoint_length_in_bytes;
m_length -= codepoint_length_in_bytes;
return *this;
}