Re: [Podofo-users] PoDoFo PdfString::Write buffer overflow

[Matthew Brincke]

Hello Mark, hello all,
> On 02 May 2018 at 10:20 Mark Rogers  wrote:
> 
> 
> Hi
> 
> That sounds good. 
> 
>  > if( pEncrypt && m_buffer.GetSize() && IsValid() ) 
> > As IsValid() contains only a NULL check on the buffer in m_buffer,
>  > the size check needs to be > 2 if ...
> 
> At the moment buffer.GetSize()=1 produces buffer underflows so changing the 
> test to 
> buffer.GetSize()>1 or buffer.GetSize()>2 will prevent heap corruption

I'm sorry I didn't get to this until now: I think it's difficult to test because
I don't see when m_buffer.GetSize() could be less than 2 in PdfString ...

> 
> The harder question is when buffer.GetSize()=2 because this may work on some 
> systems 
> although it's relying on undefined behaviour.

My maxim is: "relying on undefined behaviour is always incorrect".
> 
> Best Regards
> Mark

Best regards, mabri

--
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

Re: [Podofo-users] PoDoFo PdfString::Write buffer overflow

[Matthew Brincke]

Hello all,
> On 01 May 2018 at 00:54 Matthew Brincke  wrote:
> 
> 
> Hello Mark, hello all,
> > On 20 April 2018 at 00:09 Mark Rogers  wrote: 
> > 
> > 
> > Hi 
> > 
> > 
> > This code from PdfString::Write has a buffer overflow – it checks 
> > buffer.GetSize() > 0 then sets nInputBufferLen=GetSize()-2 which is passed 
> > to new[nInputBufferLen] and memcpy 
> 
> I'd like to contribute a fix for this (UTC tomorrow, I need to sleep soon):
> > if( pEncrypt && m_buffer.GetSize() && IsValid() ) 
> 

because I'm going to properly test the fix, I won't commit it yet, sorry, I
didn't have enough time for that.

Best regards, mabri

--
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

Re: [Podofo-users] PoDoFo PdfString::Write buffer overflow

[Matthew Brincke]

Hello Mark, hello all,
> On 20 April 2018 at 00:09 Mark Rogers  wrote: 
> 
> 
> Hi 
> 
> 
> This code from PdfString::Write has a buffer overflow – it checks 
> buffer.GetSize() > 0 then sets nInputBufferLen=GetSize()-2 which is passed 
> to new[nInputBufferLen] and memcpy 

I'd like to contribute a fix for this (UTC tomorrow, I need to sleep soon):
> if( pEncrypt && m_buffer.GetSize() && IsValid() ) 

As IsValid() contains only a NULL check on the buffer in m_buffer,
the size check needs to be > 2 if ...
> { 
>   pdf_long nInputBufferLen = m_buffer.GetSize() - 2; // Cut off the trailing 
> pair of zeros 
there is to be a trailing-zero pair at all ...
Otherwise only when there is such a pair expected: should be the Unicode case.
I mean IMHO only then should there be one, I'm going to make it so too.
In the non-Unicode case I'll check if zero-termination is needed altogether,
if it isn't the check wouldn't need to be changed, but the handling would.

>   pdf_long nUnicodeMarkerOffet = sizeof( PdfString::s_pszUnicodeMarker ); 
Of course I'd correct the typo also.

> 
> Best Regards 
>
> Mark 
> 

Best regards, mabri

P.S. Please still hold off with the rc2 for a bit (@Dominik), I'd like to 
commit Francesco Pretto's iterator API addition (13/13) and a PdfPage one 
of my own, still before the rc2 (and shouldn't the known vulnerabilities
be fixed in it also?).

--
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

[Podofo-users] PoDoFo PdfString::Write buffer overflow

[Mark Rogers]

Hi

This code from PdfString::Write has a buffer overflow – it checks 
buffer.GetSize() > 0 then sets nInputBufferLen=GetSize()-2 which is passed to 
new[nInputBufferLen] and memcpy

if( pEncrypt && m_buffer.GetSize() && IsValid() )
{
pdf_long nInputBufferLen = m_buffer.GetSize() - 2; // Cut off the trailing pair 
of zeros
pdf_long nUnicodeMarkerOffet = sizeof( PdfString::s_pszUnicodeMarker );
if( m_bUnicode )
 nInputBufferLen += nUnicodeMarkerOffet;

 char * pInputBuffer = new char[nInputBufferLen];

 if( m_bUnicode )
{
 memcpy(pInputBuffer, PdfString::s_pszUnicodeMarker, nUnicodeMarkerOffet);
  memcpy([nUnicodeMarkerOffet], m_buffer.GetBuffer(), 
nInputBufferLen - nUnicodeMarkerOffet);
}
else
 memcpy(pInputBuffer, m_buffer.GetBuffer(), nInputBufferLen);


}

If buffer.GetSize() == 1 and m_bUnicode is false then
  nInputBufferLen = -1;
   // bad_alloc or undefined behaviour when -1 sized array allocated
  char* pInputBuffer = new char[-1];
   memcpy( pInputBuffer, m_buffer.GetBuffer(), -1 );

If buffer.GetSize() == 1 and m_bUnicode is true then
  nInputBufferLen = 1;
  char* pInputBuffer = new char[1];
   // 2 bytes copied into 1 byte buffer
  memcpy( pInputBuffer, m_buffer.GetBuffer(), 2 );

If buffer.GetSize() == 2 and m_bUnicode is false then
  nInputBufferLen = 0;
  char* pInputBuffer = new char[0];
   // using pInputBuffer with size 0 is undefined behaviour 
https://stackoverflow.com/a/1087066
  memcpy( pInputBuffer, m_buffer.GetBuffer(), 0 );

If buffer.GetSize() == 2 and m_bUnicode is true then
  nInputBufferLen = 2;
  char* pInputBuffer = new char[2];
   memcpy( pInputBuffer, m_buffer.GetBuffer(), 2 );
   // first parameter is outside buffer bounds and C standard says it must 
still be a valid pointer for a zero byte copy
   // https://stackoverflow.com/a/3751937
   memcpy([2], m_buffer.GetBuffer(), 2 - 2);

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

--
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