Hello!
On Thu, Feb 23, 2023 at 12:10:27AM +0900, u5h wrote:
> # HG changeset patch
> # User Yugo Horie <u5.horie@gmail.com>
> # Date 1677077775 -32400
> # Wed Feb 22 23:56:15 2023 +0900
> # Node ID 1a9487706c6af90baf2ed770db29f689c3850721
> # Parent cffaf3f2eec8fd33605c2a37814f5ffc30371989
> core: return error when the first byte is above 0xf5 in utf-8
>
> * see https://datatracker.ietf.org/doc/html/rfc3629#section-4
>
Thanks for catching, this needs to be addressed.
The existing code accepts invalid UTF-8 sequences with the first
byte being 11111xxx (>= 0xf8), and interprets them as if the first
byte was 11110xxx (as a 4-byte sequence). This is going to
misinterpret future UTF-8 if it will be ever expanded to 5-byte
sequences (unlikely though), and might cause other problems
(including security ones, as the RFC rightfully warns, though also
unlikely in this particular case).
A more verbose commit log might be beneficial here, as well as
the one matching nginx style for commit logs. See
http://nginx.org/en/docs/contributing_changes.html for some basic
tips.
> diff -r cffaf3f2eec8 -r 1a9487706c6a src/core/ngx_string.c
> --- a/src/core/ngx_string.c Thu Feb 02 23:38:48 2023 +0300
> +++ b/src/core/ngx_string.c Wed Feb 22 23:56:15 2023 +0900
> @@ -1364,7 +1364,7 @@
>
> u = **p;
>
> - if (u >= 0xf0) {
> + if (u < 0xf5 && u >= 0xf0) {
>
> u &= 0x07;
> valid = 0xffff;
The suggested change doesn't look correct to me.
In particular, the "u == 0xf5" (as well as 0xf6 and 0xf7) case is
valid in terms of the codepoint decoding, though resulting value
will be in the invalid range as per the comment above the
function. So it does not really need any special handling, except
may be to simplify things. The interesting part here is "u >= 0xf8"
(which is complimentary to "u &= 0x07" being used in "if").
Further, as the condition is written, "u >= 0xf5" is now excluded
from the first "if", but will be matched by the "if (u >= 0xe0)"
below, and misinterpreted as a 3-byte sequence. This looks worse
than the existing behaviour.
I would rather consider something like:
diff --git a/src/core/ngx_string.c b/src/core/ngx_string.c
--- a/src/core/ngx_string.c
+++ b/src/core/ngx_string.c
@@ -1364,7 +1364,12 @@ ngx_utf8_decode(u_char **p, size_t n)
u = **p;
- if (u >= 0xf0) {
+ if (u >= 0xf8) {
+
+ (*p)++;
+ return 0xffffffff;
+
+ } else if (u >= 0xf0) {
u &= 0x07;
valid = 0xffff;
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
