diff options
author | Robin Watts <Robin.Watts@artifex.com> | 2025-03-04 16:24:33 +0000 |
---|---|---|
committer | Chris Liddell <chris.liddell@artifex.com> | 2025-03-12 08:41:18 +0000 |
commit | f14ea81e6c3d2f51593f23cdf13c4679a18f1a3f (patch) | |
tree | af4dd02da768252fff339df48ac73f0c0e5061bd | |
parent | 3885f8307726fa7611b39fa1376403406bdbd55c (diff) |
Bug 708311: Fix the fix for CVE-2024-46954.
The previous fix for CVE-2024-46954 was still failing to
spot a certain subset of 2 byte sequences as being overlong.
1 byte sequences (0xxxxxxx) encode 7 payload bits.
2 byte sequences (110xxxxx 10xxxxxx) only manage to encode 6
payload bits in the second (lowest) byte.
Thus the test for an overlong 2 byte encoding is not "is the
value of the payload bits in the first byte 0", but rather
"is the value of the payload bits in the first byte smaller
than 2".
Credit for spotting the problem and the initial version of the
fix is due to truff (https://x.com/truffzor).
Another issue spotted, and fixed here, is that it's illegal to
encode high/low surrogates within UTF-8 (as the values they
represent should be encoded directly).
Finally, we need 21 bits of coverage to get all possible
unicode values. 4 byte UTF-8 encodings give us 21 bits of data
as required, but there are values within this 21 bit range
that are not valid unicode chars. So spot these and reject
them too.
-rw-r--r-- | base/gp_utf8.c | 69 |
1 files changed, 46 insertions, 23 deletions
diff --git a/base/gp_utf8.c b/base/gp_utf8.c index b78977e37..36abab0d6 100644 --- a/base/gp_utf8.c +++ b/base/gp_utf8.c @@ -17,64 +17,87 @@ #include "gp_utf8.h" static int -decode_utf8(const char **inp, unsigned int i) +decode_utf8(const char **inp, unsigned int leading_byte) { const char *in = *inp; unsigned char c; - - if (i < 0x80) { - } else if ((i & 0xE0) == 0xC0) { - i &= 0x1F; - if (i == 0) + unsigned int codepoint; + + if (leading_byte < 0x80) { + codepoint = leading_byte; + } else if ((leading_byte & 0xE0) == 0xC0) { + codepoint = leading_byte & 0x1F; + /* Any encoded value that fails to use bit 1 upwards of this + * byte would have been better encoded in a short form. */ + if (codepoint < 2) goto fail_overlong; c = (unsigned char)*in++; if ((c & 0xC0) != 0x80) goto fail; - i = (i<<6) | (c & 0x3f); - } else if ((i & 0xF0) == 0xE0) { - i &= 0xF; - if (i == 0) + codepoint = (codepoint<<6) | (c & 0x3f); + } else if ((leading_byte & 0xF0) == 0xE0) { + codepoint = leading_byte & 0xF; + /* Any encoding that does not use any of the data bits in this + * byte would have been better encoded in a shorter form. */ + if (codepoint == 0) goto fail_overlong; c = (unsigned char)*in++; if ((c & 0xC0) != 0x80) goto fail; - i = (i<<6) | (c & 0x3f); + codepoint = (codepoint<<6) | (c & 0x3f); c = (unsigned char)*in++; if ((c & 0xC0) != 0x80) goto fail; - i = (i<<6) | (c & 0x3f); - } else if ((i & 0xF8) == 0xF0) { - i &= 0x7; - if (i == 0) + codepoint = (codepoint<<6) | (c & 0x3f); + } else if ((leading_byte & 0xF8) == 0xF0) { + codepoint = leading_byte & 0x7; + /* Any encoding that does not use any of the data bits in this + * byte would have been better encoded in a shorter form. */ + if (codepoint == 0) goto fail_overlong; c = (unsigned char)*in++; if ((c & 0xC0) != 0x80) goto fail; - i = (i<<6) | (c & 0x3f); + codepoint = (codepoint<<6) | (c & 0x3f); c = (unsigned char)*in++; if ((c & 0xC0) != 0x80) goto fail; - i = (i<<6) | (c & 0x3f); + codepoint = (codepoint<<6) | (c & 0x3f); c = (unsigned char)*in++; if ((c & 0xC0) != 0x80) goto fail; - i = (i<<6) | (c & 0x3f); + codepoint = (codepoint<<6) | (c & 0x3f); + /* Check for UTF-16 surrogates which are invalid in UTF-8 */ + if (codepoint >= 0xD800 && codepoint <= 0xDFFF) + goto fail_overlong; + /* Codepoints 0 to 0xFFFF (other than the surrogate pair + * ranges) can be coded for trivially. We can code for + * codepoints up to 0x10FFFF using surrogate pairs. + * Anything higher than that is forbidden. */ + if (codepoint > 0x10FFFF) + goto fail_overlong; } - if (0) + else { - /* If we fail, unread the last one, and return the unicode replacement char. */ + /* Longer UTF-8 encodings give more than 21 bits of data. + * The longest we can encode in utf-16 (even using surrogate + * pairs is 20 bits, so all these should fail. */ + if (0) + { + /* If we fail, unread the last one, and return the unicode replacement char. */ fail: - in--; + in--; + } fail_overlong: /* If we jump to here it's because we've detected an 'overlong' encoding. * While this seems harmless, it's actually illegal, for good reason; * this is typically an attempt to sneak stuff past security checks, like * "../" in paths. Fail this. */ - i = 0xfffd; + codepoint = 0xfffd; } *inp = in; - return i; + return codepoint; } int gp_utf8_to_uint16(unsigned short *out, const char *in) |