LibrePDF
openPDF still vulnerable to Signature wraping attack
By accident I stumbled over the fix for the signature wrapping attack (you might remember in 2019 there were some PDF vulnerabilities published) There also has been a fix for openPDF for one of the attacks. However the fix doesn't fix it correctly (imho). It becomes obvious by looking at the junit test which was added with the fix:
//this was fixed, previously true
Assertions.assertFalse(fields.signatureCoversWholeDocument(signName));
PdfPKCS7 pdfPkcs7 = fields.verifySignature(signName, "BC");
//this is wrong - openPDF should not report that the signature is valid
Assertions.assertTrue(pdfPkcs7.verify());
If you open the siwa.pdf in adobe reader it correctly reports a broken/invalid signature and so should openPDF.
The fix basically now detects that the signature doesn't cover the full document - but this does apply to all (perfectly correct) PDFs which have more than 1 signature. Because if you add a 2nd (or 3rd, 4th ...) all the previous signatures do not cover the whole document anymore. Only the last one potentially does. Now one could conclude that if there is only one signature (or the last one) and it doesn't cover the whole document, something is not right. However there are allowed changes to any signed document and if those changes have been applied in append mode then also the last (or only) signature does not cover the whole document.
Thus pdfPkcs7.verify() must return false to clearly state that something is not right with this document.
What does the openPDF community think?
Nice finding! It would be nice to have a failing test for this. I think the behavior should be similar to Acrobat Reader.
I checked the project with which we share common roots to see how they solved this. They also report that the signature is valid but does not cover the full document. So the behavior is the same like now.
I mean I am not 100% sure but when looking at the attack I would say that pk.verify() should return false instead of true.
There is an invalid ByteRange used to extend the area which is excluded by the signature. Into that area the "payload" is written. This is like an buffer overflow. First you try to place your attack code inside the executable space and then you try to execute it somehow. And the same happens here, by using a malicious XREF table to jump back...
And yes the signatures does not cover the whole document but the content has been manipulated and not added in append mode so 'not covering the whole document' is true but secondary. It is like being in a car crash and stating that the radio doesn't work anymore. That itself is correct but there are bigger problems than the radio ;-)
@mkl How do you see this?
My idea to fix this is the following: We scan the content of the /Contents<...> byte string. According to the PDF spec it is defined as:
"When ByteRange is present, the value is a hexadecimal string representing the value of the byte range digest."
So its format should be like this (abbreviated):
/Contents <308006092A8648[...]3F76416524A00000000000000000[...]0000>
But in the attack the byte range also includes other parts:
/Contents <308006092A8648[...]3F76416524A00000000000000000[...]0000>
/ByteRange [0 1633 23028 2437]
>>
endobj
1 0 obj
<</Type /Catalog/Version /1.4/Pages 22 0 R/AcroForm <</Fields [8 0 R]/SigFlags 3/DR <<
/XObject <</FRM 9 0 R>>/ProcSet [/PDF /Text /ImageB /ImageC /ImageI]>>>>>>
endobj
22 0 obj
<</Type /Pages/Kids [33 0 R]/Count 1>>
endobj
33 0 obj
<</Type /Page/MediaBox [0.0 0.0 612.0 792.0]/Parent 22 0 R/Contents 15 0 R/Resources 5 0 R/Annots [8 0 R]>>
endobj
15 0 obj
<< /Length 43 >>
stream
BT
/F1 12 Tf
(MANIPULATED now Page!) Tj
ET
endstream
endobj
So when detecting characters like '<' or '/' or keywords like 'endobj' [...] (basically all non hex characters) and error should be thrown / the document has been tempered with.
I debugged the current code however its seems that the content of /Contents is correct (only contains hex). So we have to do the parsing/tokenizing again. Theoretically we could also work directly on the bytearray since we have byte-ranges anyway... But maybe someone of you knows an elegant way to do that?
Indeed, the only sure way here is to check whether both
- the gap in the signed byte ranges contains exactly one hex encoded string and nothing else, and
- that hex encoded string equals the value of the signature field value's Contents value
are true.
I would propose not to try any short cuts here.
@mkl-public What do you think about the suggested PR? (checkByteRangeGap() method added)
Ah, I seem to have missed that notification.
If the PR still is up-to-date, I'll take a look.