Kaydet (Commit) 07ca1c58 authored tarafından Tor Lillqvist's avatar Tor Lillqvist

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
üst 5b517719
...@@ -6916,7 +6916,7 @@ bool PDFWriterImpl::finalizeSignature() ...@@ -6916,7 +6916,7 @@ bool PDFWriterImpl::finalizeSignature()
SECItem ts_cms_output; SECItem ts_cms_output;
ts_cms_output.data = 0; ts_cms_output.data = 0;
ts_cms_output.len = 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; 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); 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() ...@@ -7199,7 +7199,7 @@ bool PDFWriterImpl::finalizeSignature()
SECItem cms_output; SECItem cms_output;
cms_output.data = 0; cms_output.data = 0;
cms_output.len = 0; cms_output.len = 0;
PLArenaPool *arena = PORT_NewArena(MAX_SIGNATURE_CONTENT_LENGTH); PLArenaPool *arena = PORT_NewArena(10000);
NSSCMSEncoderContext *cms_ecx; NSSCMSEncoderContext *cms_ecx;
// Possibly it would work to even just pass NULL for the password callback function and its // Possibly it would work to even just pass NULL for the password callback function and its
...@@ -7233,11 +7233,20 @@ bool PDFWriterImpl::finalizeSignature() ...@@ -7233,11 +7233,20 @@ bool PDFWriterImpl::finalizeSignature()
} }
#endif #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; OStringBuffer cms_hexbuffer;
for (unsigned int i = 0; i < cms_output.len ; i++) for (unsigned int i = 0; i < cms_output.len ; i++)
appendHex(cms_output.data[i], cms_hexbuffer); 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 // Set file pointer to the m_nSignatureContentOffset, we're ready to overwrite PKCS7 object
nWritten = 0; nWritten = 0;
CHECK_RETURN( (osl::File::E_None == m_aFile.setPos(osl_Pos_Absolut, m_nSignatureContentOffset)) ); CHECK_RETURN( (osl::File::E_None == m_aFile.setPos(osl_Pos_Absolut, m_nSignatureContentOffset)) );
...@@ -7395,15 +7404,6 @@ bool PDFWriterImpl::finalizeSignature() ...@@ -7395,15 +7404,6 @@ bool PDFWriterImpl::finalizeSignature()
return false; 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); SAL_INFO("vcl.pdfwriter", "nTsSigLen=" << nTsSigLen);
boost::scoped_array<BYTE> pTsSig(new BYTE[nTsSigLen]); boost::scoped_array<BYTE> pTsSig(new BYTE[nTsSigLen]);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment