On quarta-feira, 1 de novembro de 2017 08:58:35 PDT Konstantin Tokarev wrote:
> > There are also no pathological cases since there is no overflow.
>
> There is overflow, try e.g. QByteArray::fromBase64() with array of size
> larger than INT_MAX / 3
Everywhere where they may be overflow, you need to be careful and detect said
overflow. Using signed or unsigned makes no difference: overflowing is bad.
> If size was unsigned such bugs wouldn't lead to crashes or potential
> security issues
Or they would. In QUtf8::convertFromUnicode:
QByteArray result(len * 3, Qt::Uninitialized);
If size() were unsigned and larger than UINT_MAX / 3, then that multiplication
would overflow. The result could potentially be a very small number.
uchar *dst = reinterpret_cast<uchar *>(const_cast<char
*>(result.constData()));
const ushort *src = reinterpret_cast<const ushort *>(uc);
const ushort *const end = src + len;
while (src != end) {
...
}
That loop never rechecks the size of the allocation. It simply assumes that
the destination buffer is large enough to encode the worst case of the UTF-8
This means the use of unsigned would not prevent issues. The loop would still
corrupt memory and could crash.
What's more, the unsigned overflow is not wrong. So you can't flag down the
issue by using a sanitiser. If we keep it as signed, then that multiplication
can be flagged and fixed.
qnumeric_p.h has add_overflow and mul_overflow for unsigned types. For a proper
signed check, you really need help from the compiler. But we can add them on
top of the unsigned ones for the ones that don't help (MSVC), by using the
implementation-defined behaviour of how an unsigned number gets converted to
signed.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
