[Libreoffice-commits] core.git: vcl/source

[Tor Lillqvist]

 vcl/source/gdi/pdfwriter_impl.cxx |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

New commits:
commit 07ca1c58d779f4daa4c84895d914780a1c814944
Author: Tor Lillqvist <tml at collabora.com>
Date:   Tue Mar 10 16:07:57 2015 +0200

    Fix signature overflow check in the NSS case
    
    We didn't actually check this correctly at all, but gladly overwrote the
    allocated part of the output PDF, thus obviously rendering it invalid.
    
    The parameter passed to PORT_NewArea is a default chunk size, not a maximum
    anything, so it was misleading, even if not wrong as such, to pass
    MAX_SIGNATURE_CONTENT_LENGTH to it. Use 10000 instead.
    
    No need to do the overflow check twice in the Win32 case.
    
    Change-Id: Ifa796dbb74b32e857f7184c1e8ada97ba124b020

diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index 838a882..55a918b 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -6916,7 +6916,7 @@ bool PDFWriterImpl::finalizeSignature()
         SECItem ts_cms_output;
         ts_cms_output.data = 0;
         ts_cms_output.len = 0;
-        PLArenaPool *ts_arena = PORT_NewArena(MAX_SIGNATURE_CONTENT_LENGTH);
+        PLArenaPool *ts_arena = PORT_NewArena(10000);
         NSSCMSEncoderContext *ts_cms_ecx;
         ts_cms_ecx = NSS_CMSEncoder_Start(ts_cms_msg, NULL, NULL, &ts_cms_output, ts_arena, PDFSigningPKCS7PasswordCallback, pass, NULL, NULL, NULL, NULL);
 
@@ -7199,7 +7199,7 @@ bool PDFWriterImpl::finalizeSignature()
     SECItem cms_output;
     cms_output.data = 0;
     cms_output.len = 0;
-    PLArenaPool *arena = PORT_NewArena(MAX_SIGNATURE_CONTENT_LENGTH);
+    PLArenaPool *arena = PORT_NewArena(10000);
     NSSCMSEncoderContext *cms_ecx;
 
     // Possibly it would work to even just pass NULL for the password callback function and its
@@ -7233,11 +7233,20 @@ bool PDFWriterImpl::finalizeSignature()
     }
 #endif
 
+    if (cms_output.len*2 > MAX_SIGNATURE_CONTENT_LENGTH)
+    {
+        SAL_WARN("vcl.pdfwriter", "Signature requires more space (" << cms_output.len*2 << ") than we reserved (" << MAX_SIGNATURE_CONTENT_LENGTH << ")");
+        NSS_CMSMessage_Destroy(cms_msg);
+        return false;
+    }
+
     OStringBuffer cms_hexbuffer;
 
     for (unsigned int i = 0; i < cms_output.len ; i++)
         appendHex(cms_output.data[i], cms_hexbuffer);
 
+    assert(cms_hexbuffer.getLength() <= MAX_SIGNATURE_CONTENT_LENGTH);
+
     // Set file pointer to the m_nSignatureContentOffset, we're ready to overwrite PKCS7 object
     nWritten = 0;
     CHECK_RETURN( (osl::File::E_None == m_aFile.setPos(osl_Pos_Absolut, m_nSignatureContentOffset)) );
@@ -7395,15 +7404,6 @@ bool PDFWriterImpl::finalizeSignature()
             return false;
         }
 
-        if (nTsSigLen*2 > MAX_SIGNATURE_CONTENT_LENGTH)
-        {
-            SAL_WARN("vcl.pdfwriter", "Signature requires more space (" << nTsSigLen*2 << ") than we reserved (" << MAX_SIGNATURE_CONTENT_LENGTH << ")");
-            CryptMsgClose(hDecodedMsg);
-            CryptMsgClose(hMsg);
-            CertFreeCertificateContext(pCertContext);
-            return false;
-        }
-
         SAL_INFO("vcl.pdfwriter", "nTsSigLen=" << nTsSigLen);
 
         boost::scoped_array<BYTE> pTsSig(new BYTE[nTsSigLen]);