[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 09 Jan 2017 09:24:51 -0800
From: Thiago Macieira <thiago@...ieira.org>
To: security@...project.org
Cc: Solar Designer <solar@...nwall.com>, oss-security@...ts.openwall.com
Subject: Re: [Security] Qt QXmlSimpleReader
On sábado, 24 de dezembro de 2016 16:18:33 PST Solar Designer wrote:
> Hi,
>
> To what extent has Qt's QXmlSimpleReader class been reviewed for
> vulnerabilities? I found only Florian Weimer's CVE-2013-4549
> "XML entity expansion denial of service", which Red Hat somehow chose
> not to fix (no intent to parse untrusted XML?) even though they got
> upstream to fix it.
It has not been at all reviewed. That class is deprecated and we have zero
resources paying attention to it.
> https://bugzilla.redhat.com/show_bug.cgi?id=955375
> http://lists.qt-project.org/pipermail/announce/2013-December/000036.html
> https://codereview.qt-project.org/#/c/71010/
> http://blog.qt.io/blog/2014/04/24/qt-4-8-6-released/
>
> Is high memory consumption for large XML files/inputs expected by users
> of this library, or is there an expectation that there would be some
> safety limits in place in the library? In my testing, for very long tag
> names or element contents, memory consumption is 4x their size - e.g.,
> about 8 GB for an almost 2 GB tag or element.
Unknown. We don't have anyone who knows the source code anymore, so we simply
can't tell you how much it may or may not cache.
The only recommended class for reading XML is QXmlStreamReader. Using any of
the classes from the QtXml library should only happen with trusted sources.
> I guess CVE-2013-4549 was worse than that? Did it result in recursive
> expansion, meaning that even a tiny input would exhaust all memory?
> (I didn't try triggering it specifically.)
Yes, exponential increase in memory usage was caused by recursive expansion.
> Then there's value.resize(), which also accepts a signed int (so the
> above code's use of signed int may have been justified, after all):
>
> http://doc.qt.io/qt-4.8/qstring.html#resize
>
> "If size is greater than the current size, the string is extended to
> make it size characters long with the extra characters added to the end.
> The new characters are uninitialized.
>
> If size is less than the current size, characters are removed from the end."
>
> No clear explanation on what will happen on a negative size, and besides
> it might also be possible to exceed 4 GB and get to positive values again.
Negative sizes are the same as zero. You can't exceed 4 GB with a signed int
in QString.
> Is there anything at higher layers, yet applicable to all published Qt's
> APIs, consistenly limiting XML inputs to below 2 GB? If so, this may be
> OK (but a comment would be nice). If not, we have a problem.
No, there's no such limitation, but many classes will impose 2 GB limits due
to array sizes. The only problem is that getting close to that limit will
already run into code we don't usually test. There are also some problems with
UB on signed overflow on Qt 4.8 and in early Qt 5 versions (I think I fixed it
in 5.4 or 5.5).
> I'd appreciate any comments, especially from Florian and from upstream.
>
> I am Bcc'ing this to the address given at
> https://wiki.qt.io/Qt_Project_Security_Policy so that they have this
> message with the Message-ID for replies to the same thread, and I will
> also notify them separately.
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
