Hi There are the CVEs in ReadXRefSubsection:
CVE-2017-6844 global buffer overflow in PdfParser::ReadXRefSubsection
CVE-2017-5855 NULL pointer dereference in PdfParser::ReadXRefSubsection
CVE-2017-5853 signed integer overflow in PdfParser.cpp
CVE-2017-6844 and CVE-2017-5853 are caused by 3 related problems:
1) The addition in if ( nFirstObject + nNumObjects > m_nNumObjects ) overflows
pdf_int64 if nFirstObject + nNumObjects > INT64_MAX. pdf_int64 is signed so the
overflow wraps to a negative number, which means m_offsets.resize() is not
always called when it should be. This leads to buffer overflow on 32-bit and
64-bit systems.
2) The nFirstObject and nNumObjects parameters ReadXRefSubsection are
pdf_int64, but nFirstObject + nNumObjects is stored in m_nNumObjects which is
long, which means the value is truncated on 32-bit systems if nFirstObject +
nNumObjects > LONG_MAX (=INT32_MAX). This leads to buffer overflow on 32-bit
systems.
3) On 32-bit systems pdf_int64 can store larger values than size_t (which is
32-bits). That means m_offsets.resize truncates nFirstObject + nNumObjects to
size_t, which overflows on 32-bit systems if nFirstObject + nNumObjects >
SIZE_MAX (=INT32_MAX or UINT32_MAX depending on compiler).
I don’t think mabri’s patch handles all of these cases because it still has the
nFirstObject + nNumObjects addition without an overflow guard
I’ve attached a patch that should resolve the 32-bit and 64-bit overflows above
(CVE-2017-6844 and CVE-2017-5853)
The patch may also resolve CVE-2017-5855, but I’ve not been able to confirm
that yet.
Best Regards
Mark
--
Mark Rogers - [email protected]
PowerMapper Software Ltd - www.powermapper.com
Registered in Scotland No 362274 Quartermile 2 Edinburgh EH3 9GL
On 08/04/2017, 01:16, "Matthew Brincke" <[email protected]> wrote:
Hi zyx, hi all,
I've addressed the below concerns, and run-tested as I could, pity
my GCC 5.2.1 didn't find a difference with/out patch with the option
-fsanitize=undefined, except for my diagnostic message ;-(. I used
the same test program as earlier. Please review the patch attached.
> zyx <[email protected]> has written on 28 February 2017 at 08:38:
>
> On Tue, 2017-02-28 at 00:14 +0100, Matthew Brincke wrote:
>
> > I haven't completed testing yet
>
> Hi,
> thanks for the patch. Just from a quick read of the proposed change:
>
> > * const pdf_int64 maxNum
> > * = static_cast(std::numeric_limits::max());
>
> As far as I know, 'long' type is architectural dependant, 32 bits on
> 32bit arch and 64 bits on 64bit arch, thus it produces different
> values. Avoiding a 'long' usage might be a general benefit.
>
> > * "(%ld)!\n",
> > * nFirstObject + nNumObjects, m_nNumObjects ); // 2nd arg is long!
>
> The %ld is incorrect for the same reason. There are defines for proper
> formats, or cast the second argument to pdf_int64 instead and use the
> format specifier as before.
>
> > * ") in this XRef table than supported by this version of PoDoFo, "
>
> This sounds odd to me, are you sure it's about what PoDoFo supports,
> not about what the standard supports? I mean, the standard suggests to
> stay in those limits even if the writer runs on a system which can
> cover more objects, to be compatible with 32-bit systems (because you
> never know on which system the reader runs).
> Bye,
> zyx
patch-CVE-2017-6844.diff
Description: patch-CVE-2017-6844.diff
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Podofo-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/podofo-users
