From f231dacde9df1c4aa5f4e0970535c4f4093364a7 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Wed, 4 Nov 2020 21:39:04 +0100 Subject: [PATCH] xmlsecurity: reject a few dangerous annotation types during pdf sig verify Change-Id: I950b49a6e7181639daf27348ddfa0f36586baa65 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105312 Tested-by: Jenkins Reviewed-by: Miklos Vajna --- .../pdfsigning/data/bad-cert-p3-stamp.pdf | Bin 0 -> 22023 bytes xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 15 ++++++ .../source/helper/pdfsignaturehelper.cxx | 49 ++++++++++++++++-- 3 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf diff --git a/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf new file mode 100644 index 0000000000000000000000000000000000000000..b30f5b03867c465c8fc59d4f2e068f4eecbb4f81 GIT binary patch literal 22023 zcmeHP30xFM*5?pZE&)+d6EcD*sL8^TJ?_ck| zs($ZPg{CGLB2Xmm9s2hlzpe82K$H&GnZ@DW(a~6cqe&4g#<7a5Be4EDg2V|P8=ylB zmZ)P{p1@Mzeuj=fEJ0v>bXhuzrga2MX2-;MD<)Z^qI>Y;?Lp!VZ6qj?Syoa7OFq7nq846qagHWTRv?TY!8_3}!IqTNMjt$QP`Ny$Y6;kQL1`aq)T%>Els6Ln106 zjwTH(n?O;B#S`KUTnv;TsKf*VO~l7DR05R#wq&_|_CL2pK$x2BUFivpV z57n$gBy=zh>#vw{t$8{cX}uX`;}c+L}i4Up<}Jn0ZH z9YN4_OmQTZt|K`tQ^(@>bv7p_N65(;E<3pxd%I^;YCs~GC;zHxk6P#lkcli~DUdHcX?<2vp48gZ(0HNpcIzdSX@sRiz|W^xOQ172+C z)yi0$WHB4emI58AaR8VLrra=DiAYEc$C3rh2yHw-Ftke$L#w4&0cl89tb#091)aT4 zs*tM`=@7CJNLB2jlwg*U6H;}E*X}}{bf_y?Gc1Z?A3=Ln+sNsGRK7VE%QPyZ6$^~D zs#p}01T;w-%?5EsOashuCX?9;MeMSoH=C@A34SBa)(ySITo`XI$mb}qwV0)J#hQhI3BWRx5-Uh8h__Z< zhu7j|Z>k7>X9!}E)>lo`HNYEz!Zv{7Ku--uOOaK_aCMSLbGT+WfTBiffH*weTF_Vm z=cFG?`4RWMJ9o^h&C8GM9ke2N>6{*U;VY_JWKJu=m$%%vEM@M79(V4({?}yz>soDI zdr>~ScwM#%MSxxf}2(LnX1zb-R`@ibLfm2r^n6c?A4|9 zVx?=z(uIR9nbB1PE5rBBYc}-4+U&!d-Z{CW$C6LZeBgA+&%9t+`TA<#i+jTT!+&{y zaZl%d?vs`tSn=GAR&&}LgGLS-JSODBpoE3hKNN2*IQ=Y-4X+BrzfW0uepi=xpZB=IfAuZLZ4%N)6LTaG8%R_ME8gN#92iuzub()WBnd(6phTUH&Oi-s(ofkk}t zQTm+#&uQ}W>)kKEeEI6KiYorZr{5cxl9>ZDdT$<2p3F3Kcx`F3vw6=A>sFvk2~=0L zNd)||O50h+ExmAIN3Zf0i1*)%Nk>bAHWuN0lUdZl{TvPd#};N9pg1NVP6^V^Y;Cre)~*)@J>`%gB8 zymG4Bp~>H@_;CHQe_Z)8=DDDfj2EMXArsl(o?hj={>aw+pdCy87P7aKSIGJ|iYHaq z42e1I`|2=i$^6T=m!$lBZbbg=zvMl0?hL=Ito3A{4pYU=rf%bu?$_r3{SAGu@82W8 zKir&OwfV%*ExSzPtKX=pPFnYmF(Eg+cFxidZhN5SQj*W*NtX_eeRhMys!Nw={pIkc z{81xlpMxVqVtuZU`n*Tp`={^qn3i(kw*}9?*y-7g=YR1z$`7cDb@6i21yOSynH){% zYfX*=8LhMQEX*}^hP%&{_XcH~t9$w0>3;jW=EFkwt{?sRs<5d6eGV)e9-PQs(9dl? zC$M^Ihv6UGy0LlZbs_lq{Hxn7=N27Z&~ep|=f}?b;*4@<%sfo@Rn5MtIXiC5-rUi* zu$caQfzzFL(q4{hd8l6)`e#j$CGdUUm&!eco%a{c>=o6#`^Fh916OUFIpgzf zhx68cwSLF+q)VSo{z>`z&8h)~i?;pv(&|?DKHcGhJuQaZdiT{0pS|z%OsC)0`$VsN z_jLSW(}_N&*>5ITTFjg^ba0n!Y<`8IWx~(TRsNK_D0lPL9m_t76BQ&-h>ZrEFf+`nRJ_7;3tF8eepItR?h|4eMGfD@J#T#j6A4PVLWr5WMO| zv@Sz9KYsAx4(^q&F+uy=_40pj=(O&eetN3R-Fu*=>#@MQon~F_^tT_=f;L=VGqPZZ zVTC2){T~Jub&)>p#t6oa-OBNRjOv>FVWXF>J%00*l&PipPluJfG&G{s$Z>LNt37WW zdTPSbQ>O9X|MO-=z?37Ot*%^i!s6S8U6x2hGv|68SYvECVYr+^oHl&=YViIJe~D-_ zYWCSzO&+bUSD^Hsmt9KDcUs@o=d|fPu7AyW-|~r_X1w`q)=Q<6O7Gq|x;1<27p4<| z`)-bSF`x5uO+ME1-i%Y7jTN7Ca-V$hsj?_JBIKmohlS6SR)6fg+_$t}^daf5Q@4Ty z&R-U>$B_hT6H%=M#*eWoX)w_#S#S^0I7|;Xm}Q#hi0ryK{c*lwy=0$tqDh=XI88G+ zgY!t>7>eTuBpKT`A{0TZq)|ukPmm~4pk6#~#C5eYpbDcd(ea$3ho8X6L6pVE$>0;K;YM5~g*suYb7hchB20v9Z3ATJ?N6>y-(lO(4Qip=vAO)=0SM+r~_R+uu2 zNDj0H+GG%gD2Y@Uk|jw}q8L@?B#uT5s3grJieN#{ISD$*ON1;66fX;^KuMCoAX)$g z;b=udf=mKkUXm#2tU@U)bQRd50Vx%r0>m^;2h{)>0oAr$Dj-WB5shsSQZ-wH3pT6L zU{gS|2^q>#ntlTvmV>W{$f3dyTm5OMI0c;SWL`15BLXip&LIH|E3cP}|Ao~O-gOLM*j3lxgCDW{o%ZwmXiU2YO z!>x^h1a^kfFd$`>h5D2v@(2+$SSC{8NJ^ztfl$HFX}AdjloTZn7g>=6sTFye0&=9v zaZrEqQJi45{)R5;#oqKS(Zj1tO6mk0?3gnBZZYgIskk`0i(D~s|3raNCi=% zSsted4uKvaRR%BtYgS-bRuNl^40*d(0PW-|v*SPd!ANk?@km{k!@ApwnR1Sb-HN9HcIr#bk+ zKot(?z!(4^1WXsKW)~zzVnFl=k`feEq9hvTFIi!bD1e<6kRb3B3l@$Cfuco{foVsU z7+IUNcmSQJI6wysvCRgNl0u0p1xpi{TyU84C{Dyhn~9Xcki(Kd=&iVFr6S)QfP`|wN;pA+eMXu$&Qm$;7XD-+Y5@M&8jpgqXs<`0ISb3NZ|+= zGES9v0aOMQoB+jDB~IfImRw+@U@Aona01qc0!~D*L?j2(1qVh{g!L25HDJj>#i3Q0 zA;C1L5|~d);AOD5U`JqzrogBul0=Gx2zG`P2(XAWAOUj_2l51SsYFTwDX1hNGGHrI zu#XJPjVgmAhF4+IP+?ku zGWho63boLJRsge+3qiORToDB;TtP|r9~R0iHXuC<8*DUXV`&f%A{dB5B9(sF+QOTF4^BJXo-pZ0&2^)yU8>@WCL~W{=Ti*$D}CA-M!2 z#Dc8-&5~d>noU_cc-KcdlDglmhS^I&7%9&zW6240?Tt(!zgUSzupWZtksvTKxFHZn zz}p{q##EIS37)6<7<<4*g{W061$(Ti;Wk7~1Y0~aRY%&x9JUaTR>S@X%fyIC4Gdch zB;A+`{KUlkD^?ha{)4Qj@ZyL#nGoR}6C7D8_!~r6l;ZFTsc62g2U#%~m3$cjXTN7g zM`@9)wEH`pjinchweEC51_>@4Ar-NaGKA-(JVmZA+z=~j6sD~8xr1|!*T`6HG1TP# z_DXdTz6N)EbZkkXAdOJ0I#J0rntBFb-n=zfXOw#e52lmxY>ANmNNeVF7eVS5lB~DvB!U87yeZ76sSZZ7f)4D`FkdGuWm!x*^G_I=$JV z=;%l$Lc$4J$5D}l=9|TPfV2|~1|25w2#|S-0I{!g)CR|B@z`jKtVX3J7;2k?Up<5K ztk%LP3>!UqbmV9<(rn4aK#q6}M;Jm8P$HsejL9mLM3{;~>_qD5Bq&9a#aO6GvQDEb zh~{Ez&*0$tz^~S}ov*@14Vj7}Z9GOw<^l{JfUJ?2qe`+=-?_qKOTMjZvVm~XV(1Fdv4*6`^5@)AVsA>+U{jvBJIk8MEH zm67+~g-5mr`NkNCp6ZfJMT%5xF1!6>N9!CuQr*gY8BG!clsA zcQVZ8{D(3B`(h34HBeAAYaOGR=?I*Qz?lq!ilXQ!oPtRu3df_ddLs6czoP*YfKeUe zfCf#1{5Pi2sO0Zy)W+){i%R3N_tQyEjsiI911=p?u#!`-8!Sx&g4q7?_JYIZ+=EtPLQwDxA z5F|hj=r*cbOV>WqjkMQp8f@bawgJo%R&ST?ml39KcW5>J**88Y>J+TRkGZ?{{6Yg&4z z&-}KlxNyq&#C=2esGh`MryL#LWmyri#82*Yw)>>xWr=S#Gg;PViu~?G!@_Ap$KQ0h z@pci`+J8w|>|tjw|{+mmGU-sZ~A!sbJ@Y0U-&Sn)yn8j&%QW+DIN07E17L- zuKAsuAe?A_~mmRlwc?Z`hSZRU;?gj(Cb26x%H^X?kvn z+B~%XyG0(9-h0xIyG|P45*Ji3L-e!T&#BzmotwSpt?qd5i=M{a&u`NWU0Ah%&CMSa z@^pEJR&6Zq2eyAOilW2*`C3eyLpf^(zV7o%8|#`*`$|`@DJ$GxIr5`B!*;s$Ek|v~ z_ghmrOL;5uu-ow6eCJ_pR>drzJ0yH%=y#vBDybgOHM_mF`a4VeS7Kd%#WyJnKTSKl zW!vPcfwRBOneE}qY^hc=@)u?FUETim&y(CfJ-Y4i*ELyP%nP&VI>NS9klh?Y)B1*}mn0<$;FnIKKC(YvC==O}jleYyZ+4D-Yj;#Ax@5s?iQ~TuUW> zc#@)B#~U)ZYxD0NXVD^*fv2usv+u_14Ap$?B=J{Q|7mGg`^HaAOK(#V+{fj`@w}$=@GGe7q-7e;mGtWd%CA2M@D?rV|hl4q~tvZ z4~&lNo7`jfo^OkidL$hzTmx})gs+BpJOyb<_&x?tWAW6L%V)=M3b+v$=7IeygGDIHu)NBSEmi0LbK(+)7MDb z9PQ`{eT|N&{8#uIAzg-+8H$R4L{u(HFCO(q z^}C{f{%2Doe+auKxSHVlLkRq#!cEfE1lJ!z;13o4{~=wE>1%|PnfkZsxKv0>NzqZ- z>%O*+TAzhS!mbL~d!@r8VTTMPn!|;*b%7(ZF$8{)u)+Vn zBqw-`xcbK_$zd`N>C%z}QCE)D;;$+nAwpJhgto&2lDM_>b+tCxC zkv^d}%~;I?Y21*2!R~MEwK)w-H5V2a>L~k8?KDNQW)VD!Bv=~u_M=FaBO#Z7hi^hh zQ*- z;Mjmd#iUb({331Vv~BCQ4d67$VuG$fGIk{F5$DNBQrjgC*BnX1w%qzAZM}EgQX?Ns zyltB})W+eDV%SL7yAJ8yku)4?VE4~tH9kH8ZbEj1)_v`42m3(>2#0<4R*SJD3vxJY zbTnFxuCwjwWrnn5=z7Dhb4bys-$!(6-n9OR}h#Ybz_&P;8=h7_MZ zQ@WkrKRUf|_c@Q#F5o5e`pPBT!Rx+_Xf3|~2Cix9Uvq^e6~LBvePia53N)DQO7ZyY zb!!&ink?!mg}A!97D;ZdAe8t%@)M^T?}&;!r-w#P_;#qP+x+n#^}?G008y`bc6S6o zBWF;&(OLvY9%w0nW|MX*0H+rz zIXe_X>PO4Q_kF_!xq~tsiUIE=4Fh-Y$h}KWKDP8JC)o);&i9Tv?QrsHp?%Q@DB*+(6L=WK*yk`>EqX` zm4Be0pPzrbj)4II?Sk3`>H^yZg$CI^p&?<~C!~j#KH7tYy4qju$K^>0-22|CnN!z$ z6;6IFT)ViKU7doRoc)|#{9NvBcYepo+1bg-%}wLC_S4+i#kHB6dyAGHp4zi!&Fda$ z-}YzCwTHg&Q2V@~w|_T&=;z-$piSF$f$ckV3J&QU+NEn)csJOFLD3A$@lkNxNkXEb zcb}xb1Jlwo24xN&^4#;V*+W*;+&ts(k#N#N@u<-yW5!OL^vbKRl};|3GIi#x*>mR3 zn_s?Q;oI-L``*%J?|-m-#hSGrulwZF^&2*Bs@(R)_8mLF+_n3wefz5p96WUR$kFOk zfB*6HnX~79I)CBHFEv-M{d)bzKX2ah`@h%ad(}=}t^lo{i=R`h(~6htu>Ql}t?qsI zM78%lQSJRtRJ-blY9Dx_+6SMg_Ms=LefWuLH;wlnRuZRH_nOB0&!E<%-+uixP~lB1R%ebCwOa)<~LJ+f)^zmW)&y?ObRx7YYRm39@}3>=<7#pCs-hZ1lUC7c>cXvaYk za3rYC@lm1_Uwf1&>>_*MAW_JXvbhr*Wy|2dJ4lq;^v(8?$T?Fco{Ft{X>x9Vuj!+D ujcPmNH}~z+`+K=ed-+Z`-EF^KFFQN^{nwU34zO#KDFa8VaHu|4=6?V-7B`6i literal 0 HcmV?d00001 diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx index 78c564b26e28..fb47b9887f15 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -402,6 +402,21 @@ CPPUNIT_TEST_FIXTURE(PDFSigningTest, testBadCertP1) rInformation.nStatus); } +CPPUNIT_TEST_FIXTURE(PDFSigningTest, testBadCertP3Stamp) +{ + std::vector aInfos + = verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + "bad-cert-p3-stamp.pdf", 1); + CPPUNIT_ASSERT(!aInfos.empty()); + SignatureInformation& rInformation = aInfos[0]; + + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 0 (SecurityOperationStatus_UNKNOWN) + // - Actual : 1 (SecurityOperationStatus_OPERATION_SUCCEEDED) + // i.e. adding a stamp annotation was not considered as a bad modification. + CPPUNIT_ASSERT_EQUAL(xml::crypto::SecurityOperationStatus::SecurityOperationStatus_UNKNOWN, + rInformation.nStatus); +} + /// Test writing a PAdES signature. CPPUNIT_TEST_FIXTURE(PDFSigningTest, testSigningCertificateAttribute) { diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx index f7427337fd1e..72f7618a2253 100644 --- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx +++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx @@ -226,8 +226,30 @@ bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument, } #if HAVE_FEATURE_PDFIUM + +/** + * Contains checksums of a PDF page, which is rendered without annotations. It also contains + * the geometry of a few dangerous annotation types. + */ +struct PageChecksum +{ + BitmapChecksum m_nPageContent; + std::vector m_aAnnotations; + bool operator==(const PageChecksum& rChecksum) const; +}; + +bool PageChecksum::operator==(const PageChecksum& rChecksum) const +{ + if (m_nPageContent != rChecksum.m_nPageContent) + { + return false; + } + + return m_aAnnotations == rChecksum.m_aAnnotations; +} + /// Collects the checksum of each page of one version of the PDF. -void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector& rPageChecksums, +void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector& rPageChecksums, int nMDPPerm) { auto pPdfium = vcl::pdf::PDFiumLibrary::get(); @@ -247,8 +269,25 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vectorgetChecksum(nMDPPerm); - rPageChecksums.push_back(nPageChecksum); + PageChecksum aPageChecksum; + aPageChecksum.m_nPageContent = pPdfPage->getChecksum(nMDPPerm); + for (int i = 0; i < pPdfPage->getAnnotationCount(); ++i) + { + std::unique_ptr pPdfAnnotation = pPdfPage->getAnnotation(i); + vcl::pdf::PDFAnnotationSubType eType = pPdfAnnotation->getSubType(); + switch (eType) + { + case vcl::pdf::PDFAnnotationSubType::Unknown: + case vcl::pdf::PDFAnnotationSubType::FreeText: + case vcl::pdf::PDFAnnotationSubType::Stamp: + case vcl::pdf::PDFAnnotationSubType::Redact: + aPageChecksum.m_aAnnotations.push_back(pPdfAnnotation->getRectangle()); + break; + default: + break; + } + } + rPageChecksums.push_back(aPageChecksum); } } #endif @@ -272,7 +311,7 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu aSignatureStream.WriteStream(rStream, nSignatureEOF); rStream.Seek(nPos); aSignatureStream.Seek(0); - std::vector aSignedPages; + std::vector aSignedPages; AnalyizeSignatureStream(aSignatureStream, aSignedPages, nMDPPerm); SvMemoryStream aFullStream; @@ -281,7 +320,7 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu aFullStream.WriteStream(rStream); rStream.Seek(nPos); aFullStream.Seek(0); - std::vector aAllPages; + std::vector aAllPages; AnalyizeSignatureStream(aFullStream, aAllPages, nMDPPerm); // Fail if any page looks different after signing and at the end. Annotations/commenting doesn't