Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121925
MIME-Version: 1.0
References: <CAMZNyFfiP5y8QAcNBCo235xG30Ra-PgCKcYfsZVuvgwy9Re-8w@mail.gmail.com>
 <CAFPFaMJeLD1WVzczjfXa4iKg0QtgKxhH0e3mzOMjsa9POf=C9w@mail.gmail.com>
 <CACsECNcbe+D=8WMa=LG-ts6_Rq1W91hneb43i07L58xC5WUt3w@mail.gmail.com>
 <CAMZNyFe0YywB5WGt3ga6s1SFOytNLFfF3=eEfMhnpWTDh24C9w@mail.gmail.com>
 <CACsECNf_+WHxFwR02n631pk5hR52eAjN1QZAw-7j+g8RQzoi3g@mail.gmail.com> <CAMZNyFd79C1TqR=K9_2tnP8aakzx5vnsOy=DFAkqZ0wxc341tw@mail.gmail.com>
In-Reply-To: <CAMZNyFd79C1TqR=K9_2tnP8aakzx5vnsOy=DFAkqZ0wxc341tw@mail.gmail.com>
Date: Tue, 5 Dec 2023 10:04:46 +0100
Message-ID: <CAPzBOBNN+d-bBJ_anEe2V3=-ikJ8z=a-H8uCQG-ky=dooFt_6g@mail.gmail.com>
To: Stefan Schiller <stefan.schiller@sonarsource.com>
Cc: Alex <alexinbeijing@gmail.com>, "G. P. B." <george.banyard@gmail.com>, 
	internals@lists.php.net, youkidearitai@gmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Inconsistency mbstring functions
From: landers.robert@gmail.com (Robert Landers)

On Tue, Dec 5, 2023 at 9:43=E2=80=AFAM Stefan Schiller via internals
<internals@lists.php.net> wrote:
>
> On Mon, Dec 4, 2023 at 8:45=E2=80=AFPM Alex <alexinbeijing@gmail.com> wro=
te:
> >
> > Stefan,
> >
> >>
> >> My biggest concern is that this quirk can cause security issues in
> >> user code. I came across this in the first place when discovering an
> >> exploitable security vulnerability in an application. From my point of
> >> view, this is not only about inconsistent behavior but also violates
> >> the documentation for specific functions like mb_strstr. I agree that
> >> a lot of mbstring operations should not be used on invalid strings,
> >> and an exception seems to be an appropriate answer despite the huge BC
> >> impact.
> >
> >
> > 1) I'm sure the vulnerable application is proprietary, but can you at l=
east tell us which mbstring functions were directly involved in this vulner=
ability, give an overview of the mechanism by which the exploit worked (scr=
ubbed of any specific details about the application), and let us know the g=
eneral nature of the resulting compromise? (i.e. denial-of-service, informa=
tion disclosure, allowing users to impersonate other users, etc)
> >
> > Real-life examples of actual security impact provide far more compellin=
g grounds for a BC break than hypothetical security impact.
>
> The case I am referring to involves mb_strpos, which is used to
> determine the index of a specific character, and mb_substr, which is
> used to extract the substring until the first occurrence of this
> character. For this specific case, this results in a Cross-Site
> Scripting (XSS) vulnerability, which would allow an attacker to
> perform actions on behalf of an authenticated user. The actual code is
> more complex, but the following lines should be a suitable
> representation of the issue:
>
> $data =3D $_GET['data'];
> $pos =3D mb_strpos($data, "<");
> $out =3D $pos ? mb_substr($data, 0, $pos) : $data;
> echo $out;
>
> The developer's assumption here is that $out should never contain an
> opening HTML tag, and it is thus safe to echo it back in the response.
> Despite the fact that e.g. htmlspecialchars should be used to encode
> the output, this assumption seems reasonable to me. However, due to
> the inconsistent behavior, an attacker can break this assumption. If
> $data, e.g., contains the sequence "\xf0AAA<b>", str_pos will consider
> the index of "<" to be 4. However, mb_substr assumes the full sequence
> to have a length of 4 (1 x 4-byte Unicode character + 3 x 1-byte
> Unicode characters). Accordingly, the full sequence, including the
> "<b>" tag, is reflected in the response.

This feels like an exact case of where mb_* functions shouldn't be
used. HTML spec very clearly states that only "<" (\x3C LESS-THAN
SIGN) is considered for html opening tags. I don't think there is a
reason to consider multibyte strings here.