From 1fb6a7d893d488d1467ec478170aa14e0227bfb4 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 8 Sep 2019 18:24:54 +0300 Subject: [PATCH] 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. --- AK/Utf8View.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/AK/Utf8View.cpp b/AK/Utf8View.cpp index 9e15f4c721..f1cfd077f8 100644 --- a/AK/Utf8View.cpp +++ b/AK/Utf8View.cpp @@ -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; }