From de52b9783864dceea3ca1c65334863ba63ab65b3 Mon Sep 17 00:00:00 2001 From: Henning Krause Date: Tue, 22 Jan 2019 19:28:50 +0100 Subject: [PATCH] Added protection for compound files with cyclic fat structure. Addresses #30, #39 and #40 --- sources/OpenMcdf/CompoundFile.cs | 31 +++++++++++++----- sources/OpenMcdf/StreamView.cs | 2 ++ .../Test/OpenMcdf.Test/CompoundFileTest.cs | 27 +++++++++++++++ .../OpenMcdf.Test/SectorCollectionTest.cs | 4 +-- .../TestFiles/corrupted-sector-chain-2.doc | Bin 0 -> 9728 bytes .../Test/TestFiles/corrupted-sector-chain.cfs | Bin 0 -> 9728 bytes .../Test/TestFiles/corrupted-sector-chain.doc | Bin 0 -> 9532 bytes 7 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 sources/Test/TestFiles/corrupted-sector-chain-2.doc create mode 100644 sources/Test/TestFiles/corrupted-sector-chain.cfs create mode 100644 sources/Test/TestFiles/corrupted-sector-chain.doc diff --git a/sources/OpenMcdf/CompoundFile.cs b/sources/OpenMcdf/CompoundFile.cs index dc12b113..ff559b49 100644 --- a/sources/OpenMcdf/CompoundFile.cs +++ b/sources/OpenMcdf/CompoundFile.cs @@ -1271,6 +1271,8 @@ List result int nextSecID = Sector.ENDOFCHAIN; + HashSet processedSectors = new HashSet(); + if (header.DIFATSectorsNumber != 0) { validationCount = (int)header.DIFATSectorsNumber; @@ -1290,6 +1292,7 @@ int nextSecID while (true && validationCount >= 0) { nextSecID = BitConverter.ToInt32(s.GetData(), GetSectorSize() - 4); + EnsureUniqueSectorIndex(nextSecID, processedSectors); // Strictly speaking, the following condition is not correct from // a specification point of view: @@ -1321,6 +1324,16 @@ int nextSecID return result; } + private static void EnsureUniqueSectorIndex(int nextSecID, HashSet processedSectors) + { + if (processedSectors.Contains(nextSecID)) + { + throw new CFCorruptedFileException("The file is corrupted."); + } + + processedSectors.Add(nextSecID); + } + /// /// Get the FAT sector chain /// @@ -1361,6 +1374,7 @@ int nextSecID //Is there any DIFAT sector containing other FAT entries ? if (difatSectors.Count > 0) { + HashSet processedSectors = new HashSet(); StreamView difatStream = new StreamView ( @@ -1404,6 +1418,7 @@ StreamView difatStream difatStream.Read(nextDIFATSectorBuffer, 0, 4); nextSecID = BitConverter.ToInt32(nextDIFATSectorBuffer, 0); + EnsureUniqueSectorIndex(nextSecID, processedSectors); nFat++; } } @@ -1425,6 +1440,7 @@ List result int nextSecID = secID; List fatSectors = GetFatSectorChain(); + HashSet processedSectors = new HashSet(); StreamView fatStream = new StreamView(fatSectors, GetSectorSize(), fatSectors.Count * GetSectorSize(), null, sourceStream); @@ -1453,10 +1469,9 @@ StreamView fatStream fatStream.Seek(nextSecID * 4, SeekOrigin.Begin); int next = fatStream.ReadInt32(); - if (next != nextSecID) - nextSecID = next; - else - throw new CFCorruptedFileException("Cyclic sector chain found. File is corrupted"); + EnsureUniqueSectorIndex(next, processedSectors); + nextSecID = next; + } @@ -1490,6 +1505,8 @@ StreamView miniFATView nextSecID = secID; + HashSet processedSectors = new HashSet(); + while (true) { if (nextSecID == Sector.ENDOFCHAIN) @@ -1509,10 +1526,8 @@ StreamView miniFATView miniFATView.Seek(nextSecID * 4, SeekOrigin.Begin); int next = miniFATReader.ReadInt32(); - if (next != nextSecID) - nextSecID = next; - else - throw new CFCorruptedFileException("Cyclic sector chain found. File is corrupted"); + nextSecID = next; + EnsureUniqueSectorIndex(nextSecID, processedSectors); } } return result; diff --git a/sources/OpenMcdf/StreamView.cs b/sources/OpenMcdf/StreamView.cs index 4f0e61ee..782c18b6 100644 --- a/sources/OpenMcdf/StreamView.cs +++ b/sources/OpenMcdf/StreamView.cs @@ -176,6 +176,8 @@ public override int Read(byte[] buffer, int offset, int count) if (nToRead != 0) { + if (secIndex > sectorChain.Count) throw new CFCorruptedFileException("The file is probably corrupted."); + Buffer.BlockCopy( sectorChain[secIndex].GetData(), 0, diff --git a/sources/Test/OpenMcdf.Test/CompoundFileTest.cs b/sources/Test/OpenMcdf.Test/CompoundFileTest.cs index ac262506..880d6296 100644 --- a/sources/Test/OpenMcdf.Test/CompoundFileTest.cs +++ b/sources/Test/OpenMcdf.Test/CompoundFileTest.cs @@ -1050,6 +1050,33 @@ public void Test_FIX_GH_38_B() } } + [TestMethod] + [ExpectedException(typeof(CFCorruptedFileException))] + public void Test_CorruptedSectorChain_Doc() + { + var f = new CompoundFile("corrupted-sector-chain.doc"); + + f.Close(); + } + + [TestMethod] + [ExpectedException(typeof(CFCorruptedFileException))] + public void Test_CorruptedSectorChain_Cfs() + { + var f = new CompoundFile("corrupted-sector-chain.cfs"); + + f.Close(); + } + + [TestMethod] + [ExpectedException(typeof(CFCorruptedFileException))] + public void Test_CorruptedSectorChain_Doc2() + { + var f = new CompoundFile("corrupted-sector-chain-2.doc"); + + f.Close(); + } + //[TestMethod] //public void Test_CORRUPTED_CYCLIC_DIFAT_VALIDATION_CHECK() //{ diff --git a/sources/Test/OpenMcdf.Test/SectorCollectionTest.cs b/sources/Test/OpenMcdf.Test/SectorCollectionTest.cs index a64fcaca..835f9df3 100644 --- a/sources/Test/OpenMcdf.Test/SectorCollectionTest.cs +++ b/sources/Test/OpenMcdf.Test/SectorCollectionTest.cs @@ -121,7 +121,7 @@ public void ItemTest() } catch (Exception ex) { - Assert.IsTrue(ex is ArgumentOutOfRangeException); + Assert.IsTrue(ex is CFException); } try @@ -130,7 +130,7 @@ public void ItemTest() } catch (Exception ex) { - Assert.IsTrue(ex is ArgumentOutOfRangeException); + Assert.IsTrue(ex is CFException); } } diff --git a/sources/Test/TestFiles/corrupted-sector-chain-2.doc b/sources/Test/TestFiles/corrupted-sector-chain-2.doc new file mode 100644 index 0000000000000000000000000000000000000000..59b4f53b25f703af0d3cba249dc5da82fe7ef81c GIT binary patch literal 9728 zcmeI1TWnNC7{}+7ZM#K;Qc6WcS)f9#SW6K_MQzKaP%Nz!MZ6%p-G#31?y0-m6nrAy zA2i}6@x??`d?GPbc z1Uw0Q;VIY$VNe|h@S_mRzfZL2e;aZreHIhEN~x;R7u}ez?9*&f`}M>J(r(7xpAGGH z)6v^Dxz7wIW2vnDCiL_xZj(D;R|N{U@M8aOS#N&3`?F^~LB*bqw~Q#3r09Qj^t9Qm zLLRqYBroY8%amVp6$YPg*$(Env`J#=1n=sPb4x~1;^;L$4;1zM$_Er(5E_^ zO>cL=JOmrN8%%O*O)vv}yziwKgo%4l)}=F|!{X zhS5T^Z6Dz@9I($JbPdt(TssF#;62K197lck8vBoC=@_X)^wmL|TO+Qwy8?A9Odslb zD%O*Fe5qrWJL<>P^x_z)qb82*1H@d*m|bvejEWyLRn&SBGAfZ#NtKr%rwTdMRCNjC zsk2scG_Gps-7#~}>fVW-QTv^!W$f2QLs-)@!wVTaMgLr=8epY7;g?q=?(vOTT)g9klw*LBBfmT4e4FI?5U*_+-#oX=}I>Un`bv z)GTVX)7wI$TwY0AOo);q))G}0SWz=p1GcbLL7<7Uc%AMl zQ9E^=ge^^ynS_GwDww(~Fj(Yl~_s^c|9jrb6X3*Ta>hn{o@do~wgRlPwLB<<` zS@0q-!<+X&v-&7#Mn3`7a0WDWD>F+bGY4P$;zfCxe0J>qxD{V=f1wey)>WEa@oXZt zB_2yey*TZo`-dY>#|E8#-LzhAhr@@?zJajVK)&@t@mOt z$X^p^Fm0+_-bt@wE|;5U+eL~fthVvPlk>f5BlhP-`u^lph3`sJUl=h(_&L(cx-aK_ zKlvBD?}LBp-a^Jq#4M!iM#wlqmt|3CjuO_ctnB`u7I=sDRX@DkuELzU+(nklD_fv! zfh)=aVY}kFqEsp?b2%-LU*}!UBFkl6F&1$Aldp4Ej7nui{$mTUzwi&vjnitDjy~TK z8vUkY+33;A<*?$&(aJ_>>*IFj$e`0YBz@NM*3@R^PTmGyYX7dCPsPdaPQKT2L+#t| zvVUKF?vqW#RZfZ}Se*<|t^QlU|fe)#)$!<-|? zmWOo_gL63l*J1V$H);{{v==#zXB{n|Hc8eEBL{u9SL6*ROS;g{$OXei-}fIze~;x=Yzli+DF?{_(w+_V1oe zl+T1<&tgg~e+B6i>MXe3wyY@kV=l+%(j}b`EJe24sszl==67b!?94ac zd~-Q-`R?19KfU+y+@H-Dcb}PJM)N^aRTS?KuJhjs=s(4c=JR*m4K)ygT9^tqz%;lKro#+So|y&zI;wI-)glY*G%mlK37gF(jh`_`Y0>gsBR!@_ z+=Y3dl!W5>qExza|ErXL(E{bztl0kk{5_fRuWy0Ie?82C2ABJXg3Yi6I$$eogHG5E4?!2~fNto4hhZo5!Y+6O9);cT80>+^VJ|!Z z`(Qsj2?rnos$(C14C00Ni!J)!h8#+t!UV5Us%!N{H|7ibG+WeuU5UPon{^N7!n@r} zY~?2Rslik{owMJBu3p7$a{KJ6K;dRy?ElT{%JqIg_t4_SAVRJH8_r0)#S`NNvAz|+H7bt%bFbeRQod1 z-5D?s!^X}=liFGv%0dtC`{@M{;-1{<1kzgrp&WR{UE#UjjLtusKVy4cBYi7w4x+;# zT4=WIA)J9j_BnvAVfvkG=U@rESGkSjsPEoj|IsWRCv}*<+HZ4f#C3O8p>CDwK|N2! z22zjCcg#XZ{kVo+94B?mB#?cGm>U_hvrdjs@gt_1TF*g75E((LJP$e5$f=>K^B7O< zb&{iTRZH)VnRzBb0CA7SfhO%gue8siys4mY^U1-op4P7L-)KY_d5Bh2NNw+m+%aIm` z3N~L8IL_c2-*u7xVye5$HrW<&o=lI2O%(?idIdCpe!(pbI8 zEm>qnWLW>*MphmZwm^N$gtLsBVa({KokHf%^W$cluqMZfMCS8a^cA}hllTyQJ!+?W zjrGf8iqro~;Zb%R&V||bqEL0zpI&(P?78m#x>K)*%nm7r+swqVDxajFIR~5;w67zC?3t`;u(TAzxLbP<41>BU}Zbp%^9r`C0A?e-oaEV zk_&H3lRlo!CDMmhgxyRy?efW08r5}&n@L5I%`rEZvT@Q)>*Y2$cJ%CPsB6P`*0t%m z^n>KPBVXHZbX}(Vm+Z3dmFzP2+lP+R6|Ti>BlVwq|9TyZ51qb_)6K?+n~bE_d9jz| zuMISsR#mRxq*p1I&(E;!B1IHd+xX$J`ChgW2MZ#7e{8Daccp14juY_B+8Fc W{^|0nS5_+9@P$V?8Fb(5r~3=RX%a~Q literal 0 HcmV?d00001 diff --git a/sources/Test/TestFiles/corrupted-sector-chain.doc b/sources/Test/TestFiles/corrupted-sector-chain.doc new file mode 100644 index 0000000000000000000000000000000000000000..193a9cb7cec0c004e2bc3ef27d2d4c2b9f461ae0 GIT binary patch literal 9532 zcmeI1Pi$OA9mnT6wzE#$bmKTCv?ny6WNfY z8luXz=y_h2Yls%7AVblxc#*2OG9@TroFI1Bho5F!Kz&q8+t&g0@@5bM4PXn{3L3#Z zU>mp>sLb|kc-J@NXVL(%O0=Fat{HU4lgZgWH=Q=-3v}5g&^KtzE<5p4 zPVi;W1-d~GH~_u^4uVI)SHU6h7)|**W$LW7Otx5D@Gf`MlikiV>T?V7)9>$U*xiCS%Y0z^iZ2 z>T1j%?br0H4^Ab5{S6AYD*eve3>3wM3ro|kiTQ6Y4G*)DA-C^P>aM6CJur-2Xxh6@f=y8=c zjw9#WYExWN;^dB)qvTd;4Vj_A8qBRR!S?fCV}jWaPDEy z?YOo;#V?t9YP|~?b;zir%Da(MkDSd^bvFZ|FDyA47!9~~++4D@_hDzu%)$*bc$;Do zTGMaJpTLWlI?t=?aU#TP859YVwsopukgFL(rq{G0x6kP%hg?2trpS@7wU>R(NV{b1 zYs9`L?Am1>4LK?rCHid0*=K8bhEQ9J6Vxnbt>d?Mr%@?iQ@@FzY1Fnv{jyfnl+%Lk z3;0eqGl%sAiWhHD60Don`nwc6WojQI7tq!wh}RSKdc_Ewx<<2{NQ_~B^J8~E11(n^ zo3O&?EDbYB<w)^pTIv)QjA4h11eaQBh(C>e8hi<49$Y4exudjwrzob8>Noqr+AEB%m1%jY zKK$Pa{gm1JExcpg3YdSrW?*~s_>UyZm#D9gf=PWuOP7;PQ+^iIy=Fme>zA~wYCqn8 zsx7hB{1v$CT2Xw!H-8jN#WNT**T7#+Qqp8w8&!b`>!TmUHG$IeZ#4tf2KSBddnsjeBS@HNBRf3iKDQbrlrZ_*trSbMJ$VU!@IX;cOmlAhXuVlmg$0+HFsDvU%yr7MLqh@PQKi zbzpm!i(C?|gWMt_b{FBTnRVxunUze_{BxigSL<5A-YZw@+Ca}RZz@;DovtZW=Kz20J!V^1LETCn?b!&MnzwJf z{YvNk;g??KXuto~Z;rvME8xw;n*f@!v?G>$mDP)lP6;Q0UXss)fRr8_+qK(1%b7Z! zN~Aeg+oQ9N@+&=ON>>=vuqsN{ou^7?3GZ?>wwy7^f9W|P!h4n;98&f&zj6t=r(0$* zwaKzqBZqtTJi7YJ|K6DH_?x3^m^r<^Kn_nBy2G9_L)_lJ;4`cEHqis>Pk+gxNAx2@ z7QKHLnN|Y@4(cYjz*;SBK@THz4R)fMB#%jHU3mc+537?r8OQuZ$q v4X%>v3WR2OSE#0{hbnHsoc>W_*K>a*BH5akNL|MEysf6CT-Pag{_fy^VyV9B literal 0 HcmV?d00001