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 - mark.rog...@powermapper.com PowerMapper Software Ltd - www.powermapper.com Registered in Scotland No 362274 Quartermile 2 Edinburgh EH3 9GL On 08/04/2017, 01:16, "Matthew Brincke" <ma...@mailbox.org> 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 <z...@litepdf.cz> 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 Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users