Adjust number of error markers emitted for truncated UTF-8 code units

In 04e59c9, I amended the UTF-8 conversion code, so that when given
invalid input, it would emit a number of errors markers harmonizing
with the WHATWG's specification of the standard UTF-8 decoding
algorithm. (Which, gentle reader of commit logs, you can find online
at https://encoding.spec.whatwg.org/#utf-8-decoder.) However, the code
in 04e59c9 was faulty in the case that a truncated UTF-8 code unit
starts with 0xF1.

Then, in dc1ba61, when making a small refactoring to a different
part of the UTF-8 conversion code, I inexplicably broke part of the
working code, causing the same fault which was already present with
truncated UTF-8 code units starting with 0xF1 to also occur with
0xF2 and 0xF3 as well. I don't remember what inane thoughts I was
thinking when I pulled off this feat of utter mental confusion.

None of these cases were covered by unit tests, by the way.

Thankfully, my trusty fuzzer picked up on this when testing the
new implementation of mb_parse_str (since the legacy UTF-8
conversion filter did not suffer from the same problem, and I was
fuzzing to find any differences in behavior between the old and
new implementations).

Fortuitously, the fuzzer also picked up another issue which was
present in 04e59c9. I was emitting only one error marker for
truncated code units starting with 0xE0 or 0xED, in cases where
the WHATWG standard indicates two should be emitted. Examples
are 0xE0 0x9F <END OF STRING> or 0xED 0xA0 <END OF STRING>.

Code units starting with 0xE0-0xED should have 3 bytes. If the
first byte is 0xE0, the second MUST be 0xA0 or greater. (Otherwise,
the codepoint could have fit in a two-byte code unit.) And if the
first byte is 0xED, the second MUST be 0x9F or less. According to
the WHATWG algorithm, step 4, if the second byte is outside the
legal range, then the decoder should emit an error... AND
reprocess the out-of-range byte. The reprocessing will then
cause another error. That's why the decoder should indicate two
errors and not one.

Author: alexdowad

ext/mbstring/libmbfl/filters/mbfilter_utf8.c

@@ -256,8 +256,11 @@ static size_t mb_utf8_to_wchar(unsigned char **in, size_t *in_len, uint32_t *buf
 				}
 			} else {
 				*out++ = MBFL_BAD_INPUT;
-				while (p < e && (*p & 0xC0) == 0x80) {
+				if (p < e && (c != 0xE0 || *p >= 0xA0) && (c != 0xED || *p < 0xA0) && (*p & 0xC0) == 0x80) {
 					p++;
+					if (p < e && (*p & 0xC0) == 0x80) {
+						p++;
+					}
 				}
 			}
 		} else if (c >= 0xF0 && c <= 0xF4) { /* 4 byte character */
@@ -285,7 +288,7 @@ static size_t mb_utf8_to_wchar(unsigned char **in, size_t *in_len, uint32_t *buf
 				*out++ = MBFL_BAD_INPUT;
 				if (p < e) {
 					unsigned char c2 = *p;
-					if ((c == 0xF0 && c2 >= 0x90) || (c == 0xF4 && c2 < 0x90)) {
+					if ((c == 0xF0 && c2 >= 0x90) || (c == 0xF4 && c2 < 0x90) || (c >= 0xF1 && c <= 0xF3)) {
 						while (p < e && (*p & 0xC0) == 0x80) {
 							p++;
 						}

ext/mbstring/libmbfl/filters/mbfilter_utf8_mobile.c

@@ -362,8 +362,11 @@ static size_t mb_mobile_utf8_to_wchar(unsigned char **in, size_t *in_len, uint32
 		} else if (c >= 0xE0 && c <= 0xEF) {
 			if ((e - p) < 2) {
 				*out++ = MBFL_BAD_INPUT;
-				while (p < e && (*p & 0xC0) == 0x80) {
+				if (p < e && (c != 0xE0 || *p >= 0xA0) && (c != 0xED || *p < 0xA0) && (*p & 0xC0) == 0x80) {
 					p++;
+					if (p < e && (*p & 0xC0) == 0x80) {
+						p++;
+					}
 				}
 				continue;
 			}
@@ -386,7 +389,7 @@ static size_t mb_mobile_utf8_to_wchar(unsigned char **in, size_t *in_len, uint32
 				*out++ = MBFL_BAD_INPUT;
 				if (p < e) {
 					unsigned char c2 = *p;
-					if ((c == 0xF0 && c2 >= 0x90) || (c == 0xF4 && c2 < 0x90)) {
+					if ((c == 0xF0 && c2 >= 0x90) || (c == 0xF4 && c2 < 0x90) || (c >= 0xF1 && c <= 0xF3)) {
 						while (p < e && (*p & 0xC0) == 0x80) {
 							p++;
 						}

ext/mbstring/tests/utf8_mobile_encodings.phpt

@@ -27,6 +27,14 @@ $badUTF8 = array(
   "\xDF" => "\x00\x00\x00%",         // should have been 2-byte
   "\xEF\xBF" => "\x00\x00\x00%",     // should have been 3-byte
   "\xF0\xBF\xBF" => "\x00\x00\x00%", // should have been 4-byte
+  "\xF1\x96" => "\x00\x00\x00%",
+  "\xF1\x96\x80" => "\x00\x00\x00%",
+  "\xF2\x94" => "\x00\x00\x00%",
+  "\xF2\x94\x80" => "\x00\x00\x00%",
+  "\xF3\x94" => "\x00\x00\x00%",
+  "\xF3\x94\x80" => "\x00\x00\x00%",
+  "\xE0\x9F" => "\x00\x00\x00%\x00\x00\x00%",
+  "\xED\xA6" => "\x00\x00\x00%\x00\x00\x00%",
 
   // Multi-byte characters which end too soon and go to ASCII
   "\xDFA" => "\x00\x00\x00%\x00\x00\x00A",

ext/mbstring/tests/utf_encodings.phpt

@@ -774,6 +774,14 @@ $invalid = array(
   "\xDF" => "\x00\x00\x00%",         // should have been 2-byte
   "\xEF\xBF" => "\x00\x00\x00%",     // should have been 3-byte
   "\xF0\xBF\xBF" => "\x00\x00\x00%", // should have been 4-byte
+  "\xF1\x96" => "\x00\x00\x00%",
+  "\xF1\x96\x80" => "\x00\x00\x00%",
+  "\xF2\x94" => "\x00\x00\x00%",
+  "\xF2\x94\x80" => "\x00\x00\x00%",
+  "\xF3\x94" => "\x00\x00\x00%",
+  "\xF3\x94\x80" => "\x00\x00\x00%",
+  "\xE0\x9F" => "\x00\x00\x00%\x00\x00\x00%",
+  "\xED\xA6" => "\x00\x00\x00%\x00\x00\x00%",
 
   // Multi-byte characters which end too soon and go to ASCII
   "\xDFA" => "\x00\x00\x00%\x00\x00\x00A",