From 688f1f7036396c282191db05765d5a3baf3af3af Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 2 Nov 2026 13:27:58 +0000 Subject: [PATCH 01/26] Fix solid 7z Deflate entry extraction --- .../Archives/SevenZip/SevenZipArchive.cs | 8 +- src/SharpCompress/Common/EntryStream.Async.cs | 2 +- src/SharpCompress/Common/EntryStream.cs | 6 +- .../SevenZip/SevenZipDeflateSolidTests.cs | 122 ++++++++++++++++++ .../Archives/7Zip.solid.Deflate.7z | Bin 0 -> 33568 bytes 5 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs create mode 100644 tests/TestArchives/Archives/7Zip.solid.Deflate.7z diff --git a/src/SharpCompress/Archives/SevenZip/SevenZipArchive.cs b/src/SharpCompress/Archives/SevenZip/SevenZipArchive.cs index f90c7e92..b541f908 100644 --- a/src/SharpCompress/Archives/SevenZip/SevenZipArchive.cs +++ b/src/SharpCompress/Archives/SevenZip/SevenZipArchive.cs @@ -188,8 +188,12 @@ public partial class SevenZipArchive : AbstractArchive @@ -54,7 +56,7 @@ public partial class EntryStream : Stream } //Need a safe standard approach to this - it's okay for compression to overreads. Handling needs to be standardised - if (_stream is IStreamStack ss) + if (_rewindDecoderOnDispose && _stream is IStreamStack ss) { if ( ss.GetStream() diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs new file mode 100644 index 00000000..328c0f21 --- /dev/null +++ b/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs @@ -0,0 +1,122 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives.SevenZip; +using SharpCompress.Readers; +using Xunit; + +namespace SharpCompress.Test.SevenZip; + +public class SevenZipDeflateSolidTests : TestBase +{ + private static readonly int[] Sizes = [1000, 70000, 100000, 200000, 500000, 1048576, 3000000]; + + // Generated with libarchive bsdtar --format 7zip --options compression=deflate. + // Each file repeats a 4096-byte xorshift sequence with a different starting offset. + private static byte[] CreateExpected(int index) + { + var block = new byte[4096]; + uint state = 0x12345678; + for (var i = 0; i < block.Length; i++) + { + state ^= state << 13; + state ^= state >> 17; + state ^= state << 5; + block[i] = (byte)state; + } + + var data = new byte[Sizes[index]]; + for (var i = 0; i < data.Length; i++) + { + data[i] = block[(i + index * 997) % block.Length]; + } + return data; + } + + [Theory] + [InlineData(512, 0)] + [InlineData(4096, 1)] + [InlineData(65536, 2)] + [InlineData(131072, 0)] + [InlineData(300000, 0)] + public void SevenZipArchive_Solid_Deflate_ExtractAllEntries(int bufferSize, int firstEntryMode) + { + using var archive = SevenZipArchive.OpenArchive( + Path.Combine(TEST_ARCHIVES_PATH, "7Zip.solid.Deflate.7z") + ); + Assert.True(archive.IsSolid); + using var reader = archive.ExtractAllEntries(); + var index = 0; + while (reader.MoveToNextEntry()) + { + Assert.Equal($"{index}.bin", reader.Entry.Key); + Assert.Equal(Sizes[index], reader.Entry.Size); + if (index == 0 && firstEntryMode == 2) + { + index++; + continue; + } + + using var stream = reader.OpenEntryStream(); + if (index == 0 && firstEntryMode == 1) + { + Assert.True(stream.Read(new byte[17], 0, 17) > 0); + } + else + { + using var output = new MemoryStream(); + stream.CopyTo(output, bufferSize); + Assert.Equal(CreateExpected(index), output.ToArray()); + } + index++; + } + Assert.Equal(Sizes.Length, index); + } + + [Theory] + [InlineData(512, 0)] + [InlineData(4096, 1)] + [InlineData(65536, 2)] + [InlineData(131072, 0)] + [InlineData(300000, 0)] + public async Task SevenZipArchive_Solid_Deflate_ExtractAllEntries_Async( + int bufferSize, + int firstEntryMode + ) + { + await using var archive = await SevenZipArchive.OpenAsyncArchive( + Path.Combine(TEST_ARCHIVES_PATH, "7Zip.solid.Deflate.7z"), + cancellationToken: CancellationToken.None + ); + Assert.True(await archive.IsSolidAsync()); + await using var reader = await archive.ExtractAllEntriesAsync(); + var index = 0; + while (await reader.MoveToNextEntryAsync(CancellationToken.None)) + { + Assert.Equal($"{index}.bin", reader.Entry.Key); + Assert.Equal(Sizes[index], reader.Entry.Size); + if (index == 0 && firstEntryMode == 2) + { + index++; + continue; + } + + await using var stream = await reader.OpenEntryStreamAsync(CancellationToken.None); + if (index == 0 && firstEntryMode == 1) + { + Assert.True( + await stream.ReadAsync(new byte[17], 0, 17, CancellationToken.None) > 0 + ); + } + else + { + using var output = new MemoryStream(); + await stream.CopyToAsync(output, bufferSize, CancellationToken.None); + Assert.Equal(CreateExpected(index), output.ToArray()); + } + index++; + } + Assert.Equal(Sizes.Length, index); + } +} diff --git a/tests/TestArchives/Archives/7Zip.solid.Deflate.7z b/tests/TestArchives/Archives/7Zip.solid.Deflate.7z new file mode 100644 index 0000000000000000000000000000000000000000..ab188ea76bc32dc8224cd77402dabf263ca27660 GIT binary patch literal 33568 zcmXr7+Ou9=hJm^I{^qTBni#-91xzw96tSqpzPb6mn&a91?}?j3m6KRan^#@E5w&%@ z!ipQ!=Vv+QZrvW`I%U?vZ_6gFkF6Hlx;ijAckA`PfB4TA+C1KO{_&1~Ew9_p|EzM? z{g?ag#s;6A)0bBKEOMOnckZ&Y)+Wj6FP9ul-~aMQ){_I*SFbXO`ucH|(RY4t{r|==bNk-h=~|CUX3HFIH~;%~@{y%|br(zWZ~aSU zXR-OS$bF;zZTIzl-&F#y96x+{%bH2sOBS*HHLkg_ODW5B@rASH>(^y%{`5+I=IPq- z;`IGjR(=zyHm&wu7khrwciCCnB9EDBe}DI4+L}v0e{6rX|J>~jUl+_c&RYCGY;nn# zFH))Jmt^cnb$98liZ98(ta^3D?aA`%_w8MEcQtoovH#u~vy0zvYIxn7q_v&zt@!)9 zt5_AS^F*R+Ef>^W__$8KJj3+EyKB$iZfCnLdBJv9?yuV$SDt!vZuYLXFRia%cfAuA zSnt%o!~ROuRihPmSNC1~{_0zQOvU5v^R_Dg)2e)b>iqU;;%DM2Z(P{3`PMbv?Y}nb z26OfM8mtT0xPejIdfgoHJ^69dykF<(O&> ze(1jZHs47~M$hPP#EUo4z5n_C8J}FUR{hW~*1lc#s&8*vdf(H@4_H<6w$`>>Ofu~C z@+p9)vLnCzW5 zXWyLSGn3;FJ5|d?)fHW7Ved40_VRmf&Z2*dn;+MjJzE^Ds#X$FTao+V@}HfPww#~- z+b?gxJU;u|lUIAy zs#lq-v&U72t$Vce_}Y4#sKOh&tagiiTW7eoI$@ES`15qH;M)AfYT|0^eSSZ`qx_!x z%m19B){k$ehuz%x_499at-lj&wSw<%^JPC9AEx;Ga2S8~lA5{|Zols2tEa60lG(<} zuN5|T;*-h)6QAES-YYq6ZIH{So>Xho?9Ev><#(=@{ZmqRkoEm*RXO{-^0o~hOJ|G7 zO)dDl@M3e6zizm$R{dn3%G*=(c39@#FzZdoUJwNVt{nQ`V(+$RqYyIS^41P5CG`iu zKfSNl`D|cd_3oDO-^bS~_iOmRJDNDVUv90ncEbDK6(EG?L3Ho^@Y=b% zzO1W^d}dyC=W)#D&GvrljQ&;3((C?x_3uW%KbKCvW>Y<}vGnDp^o81+%=(RWKmNQb z`|Rt^+Y28w{oJYFqMk9S>h6yH*Y`%)?!7(ff{6M4o&V~NG=F66%ID>*FflrPm)r5S z(tn?61?L`bn!T@nZhC#ml^x?I@$&gyIWk*$w5PcM(3QvLs;@5gn#8lQK# zO>!&bR@rrL#ha^>IjisAlH2@0?B$xu>u$DtZGR<9PSroPS7XcTBjuA<>!100a<}xu z-~Xcn+zKiRx=V(S&6gBo=o^$V(xklNA2e$Hc z`z*gZKA@b-~y;_S97_LQ9e!dEKI=NDMv`FZDIVV-K?i>uz9JMmih*K)2N zxy_rC{x$nwO5OKYBmCt?{bOq9qAZ=yavnXt?$^TOYr@-i*WF)u>|)JIF&Xd4wrNgt zN{&1GUyGTc^y+@#d99p1RqwA}+I95PJ?SgEesz@E3i0GjTI_jq!u)a;eY*|WHzpsD zVVC0Ll$!jg>MqmiRXJAWrYfJ#EIqfda`Jg=vCHL`ySiSVmaDe-p0G=E_p&X%CVBB) z+20S#uba6k)8(uE?=Lq^H(&iM{(jazsUN3bTX3?yzp*X*-hwapi};kHxpQo&K2r+=|)$ zRq&qsJN5VHOn-mjr**bdaK6=DqtBz&)4m)_}=zYIa)4yQj*`> znv3nL+TuLQv(K-b*I2kMbkFXe34e<(?-J1q&h=bnWPScm$-k%JqRGcZ>+5PBHtvoO z`L`u@;evze%Kfc=8itvb5BlEBTt0_KQb?ev{^7N!t;>1-=e=lYkbbM|cjWhn%M+Ep z|JxU0`LfSvV|?zd`8%9Gu?w?Z-M4G!|8vEyZqF@iR17N0=I;O9=3eSyys_xN?PdLK zosmKtbU$p?E9`6AUnTRW_-5+nWAEl~T>tLU+Uxq?ex;td^>g}X^<;`Pr3b?uwhSo}14el-1g zyMIohO?B^E&n%moFNfC}n{WEJ>FYsp*I(1~gf?z33OJgs`S{st(_h>BR;!)M__FKy z@xWZ8oH@CWw|94h>Jk8AIE-Ur* z?DNG%Z_oUwI`aLsQ_#IrpVpOMxUl5#ZZR9FpzOOH@8<6MduQ{R&!66Z;`N`R^X${S z*Unt;BQ8{3ImTbVR=@1W@f$0PwLGzo}#@pF* z;^ymTWc^|(op)E}&-bN2ZaQtR*;{<$@;~)y{NB6O!xu}RZOi}h<yd`hMlsSLc2C+qOyF)YHiBSLF6qduf;aVOHbSZ_m5#zw}JGwvc($c{6ymtTj$?~0#p1NjcxtGqa+jz8xf2PzQ*?&5}UaSx2%l9zJy*zi%)Hd1bS(R_6 z`n(RGx8Jrd%VS&d+UmZZo4-oFZ4L}CzN2Te_xYw7whtLsOHJ{g_U_rlF2_nOnaFzv z{a3SWCxZBl>QKD_ScpVxB?C;ysnzw3`j<2_z&_PQ5e?d~m2i#AhP7x-awuxs4Y zvzA(04t3|VEz|p-zxw&&&EfW+?cSgE_EnGl_h|hNMR`$6yZ2uY*ZZeyU9zlxZ1MI{ z+`BDr6DO{j{Mg20Lhjj~)Zkscv!fRr{Ge3d_MF3drs2Qu&O&!13?8{0+U^=vdfH;e zmBSCK)%iL&DkEPWc#!kC_?*9U!Rc=t%z>6-1z>E^lou55StzOTpH-Jhpe$)|t#a{uzOJDWD&+)(!a31_C6#g-cf zk1XBo8!b3@WBMgt?dq}{7Jpa&y4&`S>veR2%ffv7D$%&CFFn$!QTOMEE-U=d_hB~o z?P>Ls4BoB1rJi;C|Kp4EW?t^Dk-AWs)F^#ZFUIrY(wnD#U);a`UBTY6`Fs!4rF|-+ zpPc5reAr!N@4e@rPrS6>rZ@jz(Xxg6_umrv^DTGZ)%Bm2#{8NunE3tk`#lyL5C8I< zcwa`gXhmJYVH4BiqHBfhJ$DtYT=oC!>xr(p-{<6*{w=%tB0l(x?d1;%Uv@M;oi%07 z$6Y`FQJxu9E&Rx2HB$DPE^PJKEm5KD*L3ykzd>qqFb64_f){!>gt0J}YJyJMY?6 zSvP<4=V#ZrW24*la<7=WrRE>EX|dWbJ&V+RvKLntv9yHs)jvM};BlyANs6zD&PMbkace9#>um;osg-2+p*U=FP2kF;-6Yk^7?GX=R5;x=)TlrD{<@Ss^KX~x7j5~zX76P4l9!X}&hD$p zj5q&X6x7Z3HGIjhC2#uwQwrELS4zb-#+O(yr_B(_W{( zjICsM_jwz&Kee^tigFkKimS=4kH6Fg)y-b-vUcO$O?f$6OP(H{k>B+ByLIi3xOdBE z?)$g($e*9qhs(ab2u<>K__DS*l-)n(o86bmMfKNK-T8Ryq3PZ8BC~@MGVf`ZzDv6! zub#PYS@n65UD1^;AoVn+C-g1X5B_>Wa_*+=n;-ZDwO7^N+~slQRJEdd`?5E})d}%?jTL?GZLzzzD)&#T zv)=V{1)t{YZQu3%-nEoh^Y;DZ-Y@;*-H4)dH{>N#=en0Sjcd+Z9Yag@L zE;#W(^?LmDoPEW2r*f^=%eygs&#A_bucK7F_pbl5O-R=4PnF)q1=eTx^H#|RS0-&P zzdCPT_219#Kehfu7_IpGeruMERM4kO?;;hqE^ZHt-Lv4|LEY|O@xQ(oc{Hux9b2>Y zWxw0p^Sm)T?H%u?&e!4EIe&lkzI^MR3?U=UZO_*lzkHuE_s;jpDxqHMe%gNVd9tj; z{(D5QdP(i`%NJ+6FOjj&dbMWT=DMG?_m9Tem&kqcyl0c^9d@@U;s4Q=-}UmcamxyP z&U`J+6ppctR{V1LYo^nwaPRpK&dw-(-rTnC_r{>6=b`d4UXzM1EmCgp%l)sBep1Vo ze?QOdNrldxD)Dlw_bX+Tf9lb9=-|8sWV(%sviK4@HVcXzZ~sbrw;i>n`(C@(MF6=Lz|^6iyD%T7d_Y`OOIwX5{8 zd+S3>9lE!_Tt8K-@~30tj{aHdv#)8VUzU7(`Ht1E(|uO%r@~%VD1Q2|TsHmdtA9st z^WJXn&$?G|y3wM0Jzs6*1K(!{j>SgF{jN&=erLVZ-rRXleO|q;UT2>jR`k28j@zs5 zV@_yiDC@qHA2tNP*m`GoU1vx2n$lLG|2037udh6)ziNHn%{M2P+RM#ZZDSn#EX;Me zfB)|dWv8Df`)`x`Isd?lhur_x-LKA^s=oWR->oeb|H~J)yDyQuq4acjY5qCh%J{E` z?i^uMUeuSG|L@z)KH+k{d#kF9tKT%87s~a#xA6KZwY&4Syx36m@8Z_@Z94IFWp}2$ z6ndIlTK@NXW1vt2ivr`H?fwjZ&i@}p0~P}R>u>zmXiGvzXgHkpOJx~(U^T5|Mj<7 zy0Le^FjPF7b}F^)HGAU(lb!sRO8&n3F>9N6C!5WJ*&)C8bgAC3m~!;6YYLm{u0IC% zw;f|&n^d9mE|RUO;!6DEGuDRQBFBD7cXK^da!zJ@{p$G9P{XJt7QQR}dH>G7Gwo{T z3OV0jCtdTI&P=V2i(dX;oy=W7<#iCyz*jeL25(DxPRJ5TN3asGS^&(5|yZk>(22lxGdWXQlE&UQewiIKD6 gI0HL3BLkx{Gb1BwECWLW15aZu7vtkD0TTuW0G=hWLI3~& literal 0 HcmV?d00001 From d5014d3a1c32cc61bb12b0a297e4c85c7a761ce3 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 2 Nov 2026 16:11:45 +0000 Subject: [PATCH 02/26] fix legacy test --- .../SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs index 328c0f21..085abf82 100644 --- a/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs +++ b/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs @@ -102,7 +102,11 @@ public class SevenZipDeflateSolidTests : TestBase continue; } +#if LEGACY_DOTNET + using var stream = await reader.OpenEntryStreamAsync(CancellationToken.None); +#else await using var stream = await reader.OpenEntryStreamAsync(CancellationToken.None); +#endif if (index == 0 && firstEntryMode == 1) { Assert.True( From ef3a923211d6043a3b8d95c96b2808516866efc5 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 3 Nov 2026 09:50:32 +0000 Subject: [PATCH 03/26] use ODIC nuget workflow --- .github/workflows/nuget-release.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nuget-release.yml b/.github/workflows/nuget-release.yml index 345b82c4..89af78fc 100644 --- a/.github/workflows/nuget-release.yml +++ b/.github/workflows/nuget-release.yml @@ -14,6 +14,7 @@ on: permissions: contents: read + id-token: write jobs: build-and-publish: @@ -59,9 +60,16 @@ jobs: name: ${{ matrix.os }}-nuget-package path: artifacts/*.nupkg + - name: NuGet login + if: success() && matrix.os == 'windows-latest' && startsWith(github.ref, 'refs/tags/') + uses: NuGet/login@v1 + id: nuget-login + with: + user: adamhathcock + # Push to NuGet.org only for version tag pushes (Windows only) - name: Push to NuGet if: success() && matrix.os == 'windows-latest' && startsWith(github.ref, 'refs/tags/') run: dotnet run --project build/build.csproj -- push-to-nuget env: - NUGET_API_KEY: ${{ secrets.NUGET_API_KEY }} + NUGET_API_KEY: ${{ steps.nuget-login.outputs.NUGET_API_KEY }} From 9a04b51c0bc4555d4caf779c8b5f20ff32f8f8a1 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Wed, 4 Nov 2026 17:03:41 +0000 Subject: [PATCH 04/26] Reject Deflate back-references beyond initialized history --- .../Compressors/Deflate/Inflate.cs | 58 +++- .../Streams/DeflateHistoryTests.cs | 292 ++++++++++++++++++ 2 files changed, 342 insertions(+), 8 deletions(-) create mode 100644 tests/SharpCompress.Test/Streams/DeflateHistoryTests.cs diff --git a/src/SharpCompress/Compressors/Deflate/Inflate.cs b/src/SharpCompress/Compressors/Deflate/Inflate.cs index 7c972ef7..0afc6aa4 100644 --- a/src/SharpCompress/Compressors/Deflate/Inflate.cs +++ b/src/SharpCompress/Compressors/Deflate/Inflate.cs @@ -109,6 +109,9 @@ internal sealed class InflateBlocks internal object checkfn; // check function internal InflateCodes codes = new(); // if CODES, current state internal int end; // one byte after sliding window + + // Track initialized history separately from the circular read/write pointers. + internal int historyBytes; internal int[] hufts; // single malloc for tree space internal int index; // index into blens (or border) internal InfTree inftree = new(); @@ -140,6 +143,7 @@ internal sealed class InflateBlocks bitk = 0; bitb = 0; readAt = writeAt = 0; + historyBytes = 0; if (checkfn != null) { @@ -148,6 +152,8 @@ internal sealed class InflateBlocks return oldCheck; } + internal void AddHistory(int count) => historyBytes = Math.Min(end, historyBytes + count); + internal int Process(int r) { int t; // temporary storage @@ -344,6 +350,7 @@ internal sealed class InflateBlocks t = m; } _codec.InputBuffer.AsSpan(p, t).CopyTo(window.Memory.Span.Slice(q)); + AddHistory(t); p += t; n -= t; q += t; @@ -731,6 +738,7 @@ internal sealed class InflateBlocks { d.AsSpan(start, n).CopyTo(window.Memory.Span.Slice(0, n)); readAt = writeAt = n; + historyBytes = n; } // Returns true if inflate is currently at the end of a block generated @@ -1181,15 +1189,29 @@ internal sealed class InflateCodes b >>= j; k -= j; + if (dist > blocks.historyBytes) + { + mode = BADCODE; + z.Message = "invalid distance too far back"; + r = ZlibConstants.Z_DATA_ERROR; + + blocks.bitb = b; + blocks.bitk = k; + z.AvailableBytesIn = n; + z.TotalBytesIn += p - z.NextIn; + z.NextIn = p; + blocks.writeAt = q; + return blocks.Flush(r); + } + mode = COPY; goto case COPY; case COPY: // o: copying bytes in window, waiting for space f = q - dist; - while (f < 0) + if (f < 0) { - // modulo window size-"while" instead - f += blocks.end; // of "if" handles invalid distances + f += blocks.end; } while (len != 0) { @@ -1227,6 +1249,7 @@ internal sealed class InflateCodes } blocks.window.Memory.Span[q++] = blocks.window.Memory.Span[f++]; + blocks.AddHistory(1); m--; if (f == blocks.end) @@ -1273,6 +1296,7 @@ internal sealed class InflateCodes r = ZlibConstants.Z_OK; blocks.window.Memory.Span[q++] = (byte)lit; + blocks.AddHistory(1); m--; mode = START; @@ -1410,6 +1434,7 @@ internal sealed class InflateCodes k -= (tp[tp_index_t_3 + 1]); s.window.Memory.Span[q++] = (byte)tp[tp_index_t_3 + 2]; + s.AddHistory(1); m--; continue; } @@ -1465,7 +1490,27 @@ internal sealed class InflateCodes b >>= e; k -= e; + if (d > s.historyBytes) + { + z.Message = "invalid distance too far back"; + + c = z.AvailableBytesIn - n; + c = (k >> 3) < c ? k >> 3 : c; + n += c; + p -= c; + k -= c << 3; + + s.bitb = b; + s.bitk = k; + z.AvailableBytesIn = n; + z.TotalBytesIn += p - z.NextIn; + z.NextIn = p; + s.writeAt = q; + return ZlibConstants.Z_DATA_ERROR; + } + // do the copy + s.AddHistory(c); m -= c; if (q >= d) { @@ -1490,11 +1535,7 @@ internal sealed class InflateCodes else { // else offset after destination - r = q - d; - do - { - r += s.end; // force pointer in window - } while (r < 0); // covers invalid distances + r = q - d + s.end; e = s.end - r; if (c > e) { @@ -1577,6 +1618,7 @@ internal sealed class InflateCodes b >>= (tp[tp_index_t_3 + 1]); k -= (tp[tp_index_t_3 + 1]); s.window.Memory.Span[q++] = (byte)tp[tp_index_t_3 + 2]; + s.AddHistory(1); m--; break; } diff --git a/tests/SharpCompress.Test/Streams/DeflateHistoryTests.cs b/tests/SharpCompress.Test/Streams/DeflateHistoryTests.cs new file mode 100644 index 00000000..2ebe90c8 --- /dev/null +++ b/tests/SharpCompress.Test/Streams/DeflateHistoryTests.cs @@ -0,0 +1,292 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Compressors; +using SharpCompress.Compressors.Deflate; +using Xunit; + +namespace SharpCompress.Test.Streams; + +public class DeflateHistoryTests +{ + [Theory] + [InlineData(false, false, false)] + [InlineData(false, false, true)] + [InlineData(false, true, false)] + [InlineData(false, true, true)] + [InlineData(true, false, false)] + [InlineData(true, false, true)] + [InlineData(true, true, false)] + [InlineData(true, true, true)] + public async Task Deflate_Rejects_Distance_Beyond_Output_History( + bool useAsync, + bool useFastPath, + bool emptyHistory + ) + { + // Fixed Huffman blocks: either a match before any literal, or 'A' then length 3/distance 4. + byte[] compressed = emptyHistory ? [0x03, 0x02, 0x00] : [0x73, 0x04, 0x62, 0x27, 0x00]; + using var input = new MemoryStream(WithReadAhead(compressed, useFastPath)); + using var decoder = new DeflateStream(input, CompressionMode.Decompress); + using var output = new MemoryStream(); + + ZlibException exception; + if (useAsync) + { + exception = await Assert.ThrowsAsync(() => + decoder.CopyToAsync(output, 1024, CancellationToken.None) + ); + } + else + { + exception = Assert.Throws(() => decoder.CopyTo(output)); + } + + Assert.Contains("invalid distance too far back", exception.Message); + Assert.Empty(output.ToArray()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Deflate_Invalid_Distance_Does_Not_Copy_Uninitialized_History(bool useFastPath) + { + var codec = new ZlibCodec(); + codec.InitializeInflate(false); + try + { + // Seed this decoder's window directly, without relying on shared-pool reuse. + codec.istate.blocks.window.Memory.Span.Fill(0xa5); + SetInput(codec, WithReadAhead([0x73, 0x04, 0x62, 0x27, 0x00], useFastPath)); + codec.OutputBuffer = new byte[16]; + codec.AvailableBytesOut = codec.OutputBuffer.Length; + + Assert.Throws(() => codec.Inflate(FlushType.None)); + + Assert.Equal(1, codec.NextOut); + Assert.Equal((byte)'A', codec.OutputBuffer[0]); + Assert.Equal(new byte[15], codec.OutputBuffer.AsSpan(1).ToArray()); + } + finally + { + codec.EndInflate(); + } + } + + [Theory] + [InlineData(false, 3)] + [InlineData(false, 4)] + [InlineData(true, 3)] + [InlineData(true, 4)] + public void Deflate_Preset_Dictionary_Limits_Available_History( + bool useFastPath, + int dictionarySize + ) + { + var codec = new ZlibCodec(); + codec.InitializeInflate(false); + try + { + byte[] dictionary = [(byte)'A', (byte)'B', (byte)'C', (byte)'D']; + codec.istate.blocks.SetDictionary(dictionary, 0, dictionarySize); + // Length 3/distance 4, with no preceding literal. + SetInput(codec, WithReadAhead([0x03, 0x62, 0x00], useFastPath)); + codec.OutputBuffer = new byte[16]; + codec.AvailableBytesOut = codec.OutputBuffer.Length; + + if (dictionarySize < 4) + { + var exception = Assert.Throws(() => codec.Inflate(FlushType.None)); + Assert.Contains("invalid distance too far back", exception.Message); + Assert.Equal(0, codec.NextOut); + } + else + { + Assert.Equal(ZlibConstants.Z_STREAM_END, codec.Inflate(FlushType.None)); + Assert.Equal(3, codec.NextOut); + Assert.Equal( + dictionary.AsSpan(0, 3).ToArray(), + codec.OutputBuffer.AsSpan(0, 3).ToArray() + ); + } + } + finally + { + codec.EndInflate(); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Deflate_Reset_Discards_Previous_History(bool useFastPath) + { + var codec = new ZlibCodec(); + codec.InitializeInflate(false); + try + { + codec.istate.blocks.SetDictionary([1, 2, 3, 4], 0, 4); + codec.istate.Reset(); + SetInput(codec, WithReadAhead([0x03, 0x62, 0x00], useFastPath)); + codec.OutputBuffer = new byte[16]; + codec.AvailableBytesOut = codec.OutputBuffer.Length; + + var exception = Assert.Throws(() => codec.Inflate(FlushType.None)); + Assert.Contains("invalid distance too far back", exception.Message); + Assert.Equal(0, codec.NextOut); + } + finally + { + codec.EndInflate(); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Deflate_Rejects_Distance_Beyond_Window_Size(bool useFastPath) + { + var codec = new ZlibCodec(); + codec.InitializeInflate(9, false); + try + { + // Produce more than the 512-byte window, then request length 3/distance 513. + using var compressed = new MemoryStream(); + WriteStoredBlock(compressed, new byte[513]); + var match = WithReadAhead([0x03, 0x26, 0x00, 0x00], useFastPath); + compressed.Write(match, 0, match.Length); + SetInput(codec, compressed.ToArray()); + codec.OutputBuffer = new byte[1024]; + codec.AvailableBytesOut = codec.OutputBuffer.Length; + + var exception = Assert.Throws(() => codec.Inflate(FlushType.None)); + Assert.Contains("invalid distance too far back", exception.Message); + Assert.Equal(513, codec.NextOut); + } + finally + { + codec.EndInflate(); + } + } + + [Theory] + [InlineData(false, 3)] + [InlineData(true, 3)] + [InlineData(false, 32768)] + [InlineData(true, 32768)] + [InlineData(false, 32769)] + [InlineData(true, 32769)] + public async Task Deflate_History_Survives_Stored_Blocks_And_Window_Wrap( + bool useAsync, + int historySize + ) + { + var history = new byte[historySize]; + for (var i = 0; i < history.Length; i++) + { + history[i] = (byte)(i % 251); + } + + // Follow a non-final stored block with a fixed block referencing the available history. + var distance = Math.Min(historySize, 32768); + byte[] match = distance == 3 ? [0x03, 0x22, 0x00] : [0x03, 0xde, 0xff, 0x0f, 0x00]; + using var compressed = new MemoryStream(); + WriteStoredBlock(compressed, history); + var tail = WithReadAhead(match, true); + compressed.Write(tail, 0, tail.Length); + compressed.Position = 0; + + using var decoder = new DeflateStream(compressed, CompressionMode.Decompress); + using var output = new MemoryStream(); + if (useAsync) + { + await decoder.CopyToAsync(output, 137, CancellationToken.None); + } + else + { + decoder.CopyTo(output, 137); + } + + var expected = new byte[history.Length + 3]; + history.CopyTo(expected, 0); + Array.Copy(history, history.Length - distance, expected, history.Length, 3); + Assert.Equal(expected, output.ToArray()); + } + + [Theory] + [InlineData(false, 1)] + [InlineData(true, 1)] + [InlineData(false, 137)] + [InlineData(true, 137)] + public async Task Deflate_History_Survives_Overlapping_And_Partial_Matches( + bool useAsync, + int outputBufferSize + ) + { + var expected = new byte[70000]; + for (var i = 0; i < expected.Length; i++) + { + expected[i] = (byte)(i % 7); + } + using var compressed = new MemoryStream(); + using ( + var encoder = new System.IO.Compression.DeflateStream( + compressed, + System.IO.Compression.CompressionMode.Compress, + leaveOpen: true + ) + ) + { + encoder.Write(expected, 0, expected.Length); + } + compressed.Position = 0; + + using var decoder = new DeflateStream(compressed, CompressionMode.Decompress); + using var output = new MemoryStream(); + var buffer = new byte[outputBufferSize]; + int read; + while ( + ( + read = useAsync + ? await decoder.ReadAsync(buffer, 0, buffer.Length, CancellationToken.None) + : decoder.Read(buffer, 0, buffer.Length) + ) != 0 + ) + { + output.Write(buffer, 0, read); + } + + Assert.Equal(expected, output.ToArray()); + } + + private static byte[] WithReadAhead(byte[] compressed, bool useFastPath) + { + if (!useFastPath) + { + return compressed; + } + + // InflateFast requires at least ten input bytes; bytes after the final block are read-ahead. + var padded = new byte[compressed.Length + 16]; + compressed.CopyTo(padded, 0); + return padded; + } + + private static void WriteStoredBlock(Stream compressed, byte[] history) + { + compressed.WriteByte(0); + compressed.WriteByte((byte)history.Length); + compressed.WriteByte((byte)(history.Length >> 8)); + compressed.WriteByte((byte)~history.Length); + compressed.WriteByte((byte)(~history.Length >> 8)); + compressed.Write(history, 0, history.Length); + } + + private static void SetInput(ZlibCodec codec, byte[] compressed) + { + codec.InputBuffer = compressed; + codec.NextIn = 0; + codec.AvailableBytesIn = compressed.Length; + } +} From 82cc8c98c437be14fe61bdd0966890ed62d2ada4 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 6 Nov 2026 09:24:18 +0000 Subject: [PATCH 05/26] Update .NET SDK to address AOT restore vulnerability --- global.json | 2 +- src/SharpCompress/packages.lock.json | 12 +++++----- .../SharpCompress.AotSmoke/packages.lock.json | 24 +++++++++---------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/global.json b/global.json index af07787e..b14862c4 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "10.0.401", + "version": "10.0.403", "rollForward": "disable" } } diff --git a/src/SharpCompress/packages.lock.json b/src/SharpCompress/packages.lock.json index 0d73885d..f75f4d5b 100644 --- a/src/SharpCompress/packages.lock.json +++ b/src/SharpCompress/packages.lock.json @@ -321,9 +321,9 @@ "net10.0": { "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[10.0.12, )", - "resolved": "10.0.12", - "contentHash": "xi+BDjFpW+Sb+MHFHaH6Y/gV9I8BluFwRXc1QyCdoZbIK26eNiBeFuMTe/FMwc33G1wdHCyDg7CVTmb8OdQrMQ==" + "requested": "[10.0.14, )", + "resolved": "10.0.14", + "contentHash": "YlTamXppEQls6DYWt7RdUuwWDb0lyw/vXCaGeQpLCpsLmfwGuCkOz/vIGgMTV7ECsJcavkCOkxZXv9s8rdAHkQ==" }, "Microsoft.NETFramework.ReferenceAssemblies": { "type": "Direct", @@ -441,9 +441,9 @@ "net8.0": { "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.31, )", - "resolved": "8.0.31", - "contentHash": "OJdAGATV2QoVWL11zlDLEluA06q8rK+cgZR47ohrO8V+Qj5z9dlvN7bG9/0e5t0IOe+yD8tREC0bpsEgDGrzIQ==" + "requested": "[8.0.33, )", + "resolved": "8.0.33", + "contentHash": "dG9SM/0/zOTIE1UvSrveD1iDyEJjThqPqSnreExgqQWSoGWvze0esDi0faRk3C44IO1kqCeEX6AeCxWmBOppfw==" }, "Microsoft.NETFramework.ReferenceAssemblies": { "type": "Direct", diff --git a/tests/SharpCompress.AotSmoke/packages.lock.json b/tests/SharpCompress.AotSmoke/packages.lock.json index dc98e9bd..3d5f9e59 100644 --- a/tests/SharpCompress.AotSmoke/packages.lock.json +++ b/tests/SharpCompress.AotSmoke/packages.lock.json @@ -4,15 +4,15 @@ "net10.0": { "Microsoft.DotNet.ILCompiler": { "type": "Direct", - "requested": "[10.0.12, )", - "resolved": "10.0.12", - "contentHash": "AawF393Q+VkdrnrnI1gu612zh5iqpa1AGSvnKCQ3IkMgSKJXIQbO1sXYRgEzOU4f0cPZ6MaCCF29xZSGWlmbuQ==" + "requested": "[10.0.14, )", + "resolved": "10.0.14", + "contentHash": "CDQ/+XRT1JT/0u87j9exR1AociehKUzxae4KCOWxCQ1PIKbYS0F8ovxaCrqRlkylGmbhywKxV/zbbrEhWa5NXg==" }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[10.0.12, )", - "resolved": "10.0.12", - "contentHash": "xi+BDjFpW+Sb+MHFHaH6Y/gV9I8BluFwRXc1QyCdoZbIK26eNiBeFuMTe/FMwc33G1wdHCyDg7CVTmb8OdQrMQ==" + "requested": "[10.0.14, )", + "resolved": "10.0.14", + "contentHash": "YlTamXppEQls6DYWt7RdUuwWDb0lyw/vXCaGeQpLCpsLmfwGuCkOz/vIGgMTV7ECsJcavkCOkxZXv9s8rdAHkQ==" }, "Microsoft.NETFramework.ReferenceAssemblies": { "type": "Direct", @@ -76,17 +76,17 @@ "net10.0/linux-x64": { "Microsoft.DotNet.ILCompiler": { "type": "Direct", - "requested": "[10.0.12, )", - "resolved": "10.0.12", - "contentHash": "AawF393Q+VkdrnrnI1gu612zh5iqpa1AGSvnKCQ3IkMgSKJXIQbO1sXYRgEzOU4f0cPZ6MaCCF29xZSGWlmbuQ==", + "requested": "[10.0.14, )", + "resolved": "10.0.14", + "contentHash": "CDQ/+XRT1JT/0u87j9exR1AociehKUzxae4KCOWxCQ1PIKbYS0F8ovxaCrqRlkylGmbhywKxV/zbbrEhWa5NXg==", "dependencies": { - "runtime.linux-x64.Microsoft.DotNet.ILCompiler": "10.0.12" + "runtime.linux-x64.Microsoft.DotNet.ILCompiler": "10.0.14" } }, "runtime.linux-x64.Microsoft.DotNet.ILCompiler": { "type": "Transitive", - "resolved": "10.0.12", - "contentHash": "wAa+LAE6+Bkd+pEJvGdnQU8PZQRzvhurpQ4hzsOTpRuedIq1kunJcz86xBCBlzsc+QlVdCKVUPzEfB3uYVB7Gw==" + "resolved": "10.0.14", + "contentHash": "RbHp9HXSvTnYCy1X8LUQmL0LcW++jP8Crqvt6ZjSH2MXqJ7w3FBHS9sUf1iFuCD6ztYdAQNd/QgMpDGZE/aRIA==" } } } From 990186acc53ee0014496f3f46c5da8dd6c50baa4 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 6 Nov 2026 16:03:00 +0000 Subject: [PATCH 06/26] Fix writable archive duplicate entry checks --- .../Archives/AbstractWritableArchive.Async.cs | 50 +++++-- .../Archives/AbstractWritableArchive.cs | 58 +++++++-- .../Tar/TarArchiveDuplicateTests.cs | 42 ++++++ .../Zip/ZipArchiveDuplicateTests.cs | 122 ++++++++++++++++++ 4 files changed, 250 insertions(+), 22 deletions(-) create mode 100644 tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs create mode 100644 tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs index 67b8dffc..4b29c4ca 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using SharpCompress.Common; @@ -14,6 +15,28 @@ public abstract partial class AbstractWritableArchive where TVolume : IVolume where TOptions : IWriterOptions { + public override IAsyncEnumerable EntriesAsync => GetCurrentEntriesAsync(); + + private async IAsyncEnumerable GetCurrentEntriesAsync( + [EnumeratorCancellation] CancellationToken cancellationToken = default + ) + { + await foreach ( + var entry in OldEntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) + ) + { + yield return entry; + } + foreach (var entry in newEntries) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!removedEntries.Contains(entry)) + { + yield return entry; + } + } + } + // Async property moved from main file private IAsyncEnumerable OldEntriesAsync => base.EntriesAsync.Where(x => !removedEntries.Contains(x)); @@ -22,9 +45,12 @@ public abstract partial class AbstractWritableArchive { if (pauseRebuilding) { + // Disposing the synchronous pause must not perform synchronous I/O for async callers. + asyncRebuildPending = true; return; } hasModifications = true; + asyncRebuildPending = false; newEntries.RemoveAll(v => removedEntries.Contains(v)); modifiedEntries.Clear(); await foreach (var entry in OldEntriesAsync.ConfigureAwait(false)) @@ -49,20 +75,21 @@ public abstract partial class AbstractWritableArchive ) { await foreach ( - var entry in EntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) + var entry in OldEntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) ) { - var path = entry.Key; - if (path is null) + if (entry.Key is { } path && KeysMatch(path, key)) { - continue; + return true; } - var p = path.Replace('/', '\\'); - if (p.Length > 0 && p[0] == '\\') + } + foreach (var entry in newEntries) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!removedEntries.Contains(entry) && entry.Key is { } path && KeysMatch(path, key)) { - p = p.Substring(1); + return true; } - return string.Equals(p, key, StringComparison.OrdinalIgnoreCase); } return false; } @@ -116,9 +143,12 @@ public abstract partial class AbstractWritableArchive CancellationToken cancellationToken = default ) { + var activeNewEntries = newEntries.Where(x => !removedEntries.Contains(x)).ToList(); //reset streams of new entries - newEntries.Cast().ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); - await SaveToAsync(stream, options, OldEntriesAsync, newEntries, cancellationToken) + activeNewEntries + .Cast() + .ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); + await SaveToAsync(stream, options, OldEntriesAsync, activeNewEntries, cancellationToken) .ConfigureAwait(false); } diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.cs b/src/SharpCompress/Archives/AbstractWritableArchive.cs index 59d55771..42ff5d5d 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.cs @@ -32,7 +32,10 @@ public abstract partial class AbstractWritableArchive public void Dispose() { archive.pauseRebuilding = false; - archive.RebuildModifiedCollection(); + if (!archive.asyncRebuildPending) + { + archive.RebuildModifiedCollection(); + } } } @@ -42,6 +45,7 @@ public abstract partial class AbstractWritableArchive private readonly List modifiedEntries = new(); private bool hasModifications; private bool pauseRebuilding; + private bool asyncRebuildPending; internal AbstractWritableArchive(ArchiveType type) : base(type) { } @@ -53,6 +57,10 @@ public abstract partial class AbstractWritableArchive { get { + if (asyncRebuildPending && !pauseRebuilding) + { + RebuildModifiedCollection(); + } if (hasModifications) { return modifiedEntries; @@ -70,6 +78,7 @@ public abstract partial class AbstractWritableArchive return; } hasModifications = true; + asyncRebuildPending = false; newEntries.RemoveAll(v => removedEntries.Contains(v)); modifiedEntries.Clear(); modifiedEntries.AddRange(OldEntries.Concat(newEntries)); @@ -126,22 +135,44 @@ public abstract partial class AbstractWritableArchive private bool DoesKeyMatchExisting(string key) { - foreach (var path in Entries.Select(x => x.Key)) + foreach ( + var path in OldEntries + .Concat(newEntries) + .Where(x => !removedEntries.Contains(x)) + .Select(x => x.Key) + ) { - if (path is null) + if (path is not null && KeysMatch(path, key)) { - continue; + return true; } - var p = path.Replace('/', '\\'); - if (p.Length > 0 && p[0] == '\\') - { - p = p.Substring(1); - } - return string.Equals(p, key, StringComparison.OrdinalIgnoreCase); } return false; } + private bool KeysMatch(string left, string right) => + string.Equals(NormalizeKey(left), NormalizeKey(right), StringComparison.OrdinalIgnoreCase); + + private string NormalizeKey(string key) + { + key = key.Replace('\\', '/'); + if (Type == ArchiveType.Zip) + { + // ZipWriter removes drive prefixes and surrounding slashes when writing names. +#if LEGACY_DOTNET + var colon = key.IndexOf(':'); +#else + var colon = key.IndexOf(':', StringComparison.Ordinal); +#endif + if (colon >= 0) + { + key = key.Substring(colon + 1); + } + return key.Trim('/'); + } + return key.StartsWith('/') ? key.Substring(1) : key; + } + ValueTask IWritableAsyncArchive.RemoveEntryAsync(IArchiveEntry entry) => RemoveEntryAsync((TEntry)entry); @@ -180,9 +211,12 @@ public abstract partial class AbstractWritableArchive public void SaveTo(Stream stream, TOptions options) { + var activeNewEntries = newEntries.Where(x => !removedEntries.Contains(x)).ToList(); //reset streams of new entries - newEntries.Cast().ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); - SaveTo(stream, options, OldEntries, newEntries); + activeNewEntries + .Cast() + .ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); + SaveTo(stream, options, OldEntries, activeNewEntries); } protected TEntry CreateEntry( diff --git a/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs b/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs new file mode 100644 index 00000000..6d4aa083 --- /dev/null +++ b/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs @@ -0,0 +1,42 @@ +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using SharpCompress.Archives.Tar; +using SharpCompress.Common; +using Xunit; + +namespace SharpCompress.Test.Tar; + +public class TarArchiveDuplicateTests +{ + [Fact] + public void Tar_AddEntry_RejectsDuplicateAfterFirstEntry() + { + using var archive = TarArchive.CreateArchive(); + using var stream = new MemoryStream(); + archive.AddEntry("other", stream, false); + archive.AddEntry("dir/file", stream, false); + Assert.Throws(() => archive.AddEntry("DIR\\FILE", stream, false)); + var entry = archive.Entries.Single(x => x.Key == "dir/file"); + archive.RemoveEntry(entry); + archive.AddEntry("dir/file", stream, false); + } + + [Fact] + public async Task Tar_Async_ChecksPendingEntriesWhileRebuildingIsPaused() + { + await using var archive = await TarArchive.CreateAsyncArchive(); + using var stream = new MemoryStream(); + using (archive.PauseEntryRebuilding()) + { + await archive.AddEntryAsync("other", stream, false); + var entry = await archive.AddEntryAsync("dir/file", stream, false); + await Assert.ThrowsAsync(async () => + await archive.AddEntryAsync("DIR\\FILE", stream, false) + ); + await archive.RemoveEntryAsync(entry); + await archive.AddEntryAsync("dir/file", stream, false); + } + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + } +} diff --git a/tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs b/tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs new file mode 100644 index 00000000..ea333644 --- /dev/null +++ b/tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs @@ -0,0 +1,122 @@ +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using SharpCompress.Archives.Zip; +using SharpCompress.Common; +using SharpCompress.Readers; +using SharpCompress.Writers.Zip; +using Xunit; + +namespace SharpCompress.Test.Zip; + +public class ZipArchiveDuplicateTests +{ + [Theory] + [InlineData("dir/file.txt", "DIR/FILE.TXT")] + [InlineData("dir/file.txt", "dir\\file.txt")] + [InlineData("file.txt", "C:/file.txt")] + [InlineData("file.txt", "//file.txt/")] + public void Zip_AddEntry_RejectsDuplicateAfterFirstEntry(string first, string duplicate) + { + using var archive = ZipArchive.CreateArchive(); + using var stream = new MemoryStream(); + archive.AddEntry("other", stream, false); + archive.AddEntry(first, stream, false); + Assert.Throws(() => archive.AddEntry(duplicate, stream, false)); + } + + [Fact] + public void Zip_PausedRebuilding_ChecksPendingAndRemovedEntries() + { + using var archive = ZipArchive.CreateArchive(); + using var stream = new MemoryStream(); + using (archive.PauseEntryRebuilding()) + { + var entry = archive.AddEntry("dir/file", stream, false); + Assert.Throws(() => archive.AddEntry("DIR\\FILE", stream, false)); + archive.RemoveEntry(entry); + archive.AddEntry("dir/file", stream, false); + archive.AddDirectoryEntry("folder"); + Assert.Throws(() => archive.AddDirectoryEntry("FOLDER/")); + } + Assert.Equal(2, archive.Entries.Count()); + } + + [Fact] + public async Task Zip_Async_ChecksPendingEntriesAndExposesModifications() + { + await using var archive = await ZipArchive.CreateAsyncArchive(); + using var stream = new MemoryStream(); + await archive.AddEntryAsync("other", stream, false); + var entry = await archive.AddEntryAsync("dir/file", stream, false); + await Assert.ThrowsAsync(async () => + await archive.AddEntryAsync("DIR\\FILE", stream, false) + ); + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + await archive.RemoveEntryAsync(entry); + Assert.Single(await archive.EntriesAsync.ToListAsync()); + await archive.AddEntryAsync("dir/file", stream, false); + using (archive.PauseEntryRebuilding()) + { + await archive.AddDirectoryEntryAsync("folder"); + await Assert.ThrowsAsync(async () => + await archive.AddDirectoryEntryAsync("FOLDER/") + ); + } + Assert.Equal(3, (await archive.EntriesAsync.ToListAsync()).Count); + } + + [Fact] + public async Task Zip_ExistingEntries_AreCheckedSynchronouslyAndAsynchronously() + { + using var data = new MemoryStream(); + using var source = new MemoryStream(); + using (var archive = ZipArchive.CreateArchive()) + { + archive.AddEntry("other", source, false); + archive.AddEntry("dir/file", source, false); + archive.SaveTo( + data, + new ZipWriterOptions(CompressionType.Deflate) { LeaveStreamOpen = true } + ); + } + data.Position = 0; + using ( + var archive = ZipArchive.OpenArchive(data, new ReaderOptions { LeaveStreamOpen = true }) + ) + { + Assert.Throws(() => archive.AddEntry("DIR\\FILE", source, false)); + } + data.Position = 0; + await using ( + var archive = await ZipArchive.OpenAsyncArchive( + data, + new ReaderOptions { LeaveStreamOpen = true } + ) + ) + { + await Assert.ThrowsAsync(async () => + await archive.AddEntryAsync("DIR\\FILE", source, false) + ); + var entry = await archive.EntriesAsync.SingleAsync(x => x.Key == "dir/file"); + using (archive.PauseEntryRebuilding()) + { + await archive.RemoveEntryAsync(entry); + var temporary = await archive.AddEntryAsync("temporary", source, false); + await archive.RemoveEntryAsync(temporary); + await archive.AddEntryAsync("dir/file", source, false); + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + } + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + using var output = new MemoryStream(); + await archive.SaveToAsync( + output, + new ZipWriterOptions(CompressionType.Deflate) { LeaveStreamOpen = true } + ); + output.Position = 0; + using var saved = ZipArchive.OpenArchive(output); + Assert.Equal(2, saved.Entries.Count()); + Assert.DoesNotContain(saved.Entries, x => x.Key == "temporary"); + } + } +} From fa3dff056a5b55ba168fe27fcfd18f79ca43f822 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 11:16:14 +0000 Subject: [PATCH 07/26] Reject stalled BZip2 decoder states after corruption --- .../BZip2/CBZip2InputStream.Async.cs | 12 +- .../Compressors/BZip2/CBZip2InputStream.cs | 13 +- .../BZip2/BZip2StreamAsyncTests.cs | 37 ++++ .../BZip2/BZip2StreamTests.cs | 2 + .../Zip/ZipBZip2CorruptionTests.cs | 167 ++++++++++++++++++ 5 files changed, 217 insertions(+), 14 deletions(-) create mode 100644 tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs diff --git a/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.Async.cs b/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.Async.cs index c1053216..6fcbcdbd 100644 --- a/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.Async.cs +++ b/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.Async.cs @@ -21,26 +21,24 @@ internal partial class CBZip2InputStream var retChar = currentChar; switch (currentState) { - case START_BLOCK_STATE: - break; - case RAND_PART_A_STATE: - break; case RAND_PART_B_STATE: await SetupRandPartBAsync(cancellationToken).ConfigureAwait(false); break; case RAND_PART_C_STATE: await SetupRandPartCAsync(cancellationToken).ConfigureAwait(false); break; - case NO_RAND_PART_A_STATE: - break; case NO_RAND_PART_B_STATE: await SetupNoRandPartBAsync(cancellationToken).ConfigureAwait(false); break; case NO_RAND_PART_C_STATE: await SetupNoRandPartCAsync(cancellationToken).ConfigureAwait(false); break; + case START_BLOCK_STATE: + case RAND_PART_A_STATE: + case NO_RAND_PART_A_STATE: default: - break; + // Match ReadByte(): retries after a decoding error must not generate endless output. + throw new InvalidFormatException("BZip2: invalid decoder state"); } return retChar; } diff --git a/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.cs b/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.cs index bcc0c73a..7759bb12 100644 --- a/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.cs +++ b/src/SharpCompress/Compressors/BZip2/CBZip2InputStream.cs @@ -263,26 +263,25 @@ internal partial class CBZip2InputStream : Stream var retChar = currentChar; switch (currentState) { - case START_BLOCK_STATE: - break; - case RAND_PART_A_STATE: - break; case RAND_PART_B_STATE: SetupRandPartB(); break; case RAND_PART_C_STATE: SetupRandPartC(); break; - case NO_RAND_PART_A_STATE: - break; case NO_RAND_PART_B_STATE: SetupNoRandPartB(); break; case NO_RAND_PART_C_STATE: SetupNoRandPartC(); break; + case START_BLOCK_STATE: + case RAND_PART_A_STATE: + case NO_RAND_PART_A_STATE: default: - break; + // These transitional states can remain after a decoding error. A retry (including + // entry disposal) must fail rather than return the same byte indefinitely. + throw new InvalidFormatException("BZip2: invalid decoder state"); } return retChar; } diff --git a/tests/SharpCompress.Test/BZip2/BZip2StreamAsyncTests.cs b/tests/SharpCompress.Test/BZip2/BZip2StreamAsyncTests.cs index d5b2ef78..9c3c65f4 100644 --- a/tests/SharpCompress.Test/BZip2/BZip2StreamAsyncTests.cs +++ b/tests/SharpCompress.Test/BZip2/BZip2StreamAsyncTests.cs @@ -2,7 +2,9 @@ using System; using System.Buffers; using System.IO; using System.Text; +using System.Threading; using System.Threading.Tasks; +using SharpCompress.Common; using SharpCompress.Compressors.BZip2; using SharpCompress.Test.Mocks; using Xunit; @@ -11,6 +13,41 @@ namespace SharpCompress.Test.BZip2; public class BZip2StreamAsyncTests { + [Fact] + public async Task BZip2Stream_Throws_On_Read_After_Corrupt_Checksum_Async() + { + using var compressed = new MemoryStream(); + await using ( + var writer = await BZip2Stream.CreateAsync( + new AsyncOnlyStream(compressed), + SharpCompress.Compressors.CompressionMode.Compress, + false, + leaveOpen: true, + cancellationToken: CancellationToken.None + ) + ) + { + var data = Encoding.ASCII.GetBytes("BZip2 checksum validation test data."); + await writer.WriteAsync(data, 0, data.Length, CancellationToken.None); + } + + var bytes = compressed.ToArray(); + bytes[^5] ^= 1; + await using var stream = await BZip2Stream.CreateAsync( + new AsyncOnlyStream(new MemoryStream(bytes)), + SharpCompress.Compressors.CompressionMode.Decompress, + false, + cancellationToken: CancellationToken.None + ); + + await Assert.ThrowsAsync(() => + stream.CopyToAsync(Stream.Null, 81920, CancellationToken.None) + ); + await Assert.ThrowsAsync(() => + stream.ReadAsync(new byte[1], 0, 1, CancellationToken.None) + ); + } + private byte[] CreateTestData(int size) { var data = new byte[size]; diff --git a/tests/SharpCompress.Test/BZip2/BZip2StreamTests.cs b/tests/SharpCompress.Test/BZip2/BZip2StreamTests.cs index dc603878..920a89f2 100644 --- a/tests/SharpCompress.Test/BZip2/BZip2StreamTests.cs +++ b/tests/SharpCompress.Test/BZip2/BZip2StreamTests.cs @@ -24,6 +24,8 @@ public class BZip2StreamTests using var output = new MemoryStream(); Assert.Throws(() => stream.CopyTo(output)); + Assert.Throws(() => stream.ReadByte()); + Assert.Throws(() => stream.Read(new byte[1], 0, 1)); } // A stream that ends exactly where a block header is expected (no stream footer) is the shape a caller diff --git a/tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs b/tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs new file mode 100644 index 00000000..d9990ec7 --- /dev/null +++ b/tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs @@ -0,0 +1,167 @@ +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Readers; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.Zip; + +public class ZipBZip2CorruptionTests : TestBase +{ + [Theory] + [InlineData(240, 0x8d, 0x5a, false)] + [InlineData(11318, 0x35, 0xa8, false)] + [InlineData(41884, 0x12, 0x89, false)] + [InlineData(240, 0x8d, 0x5a, true)] + [InlineData(11318, 0x35, 0xa8, true)] + [InlineData(41884, 0x12, 0x89, true)] + public void Zip_BZip2_Reader_Throws_On_Corruption( + int offset, + byte original, + byte replacement, + bool skipEntry + ) + { + using var stream = new ForwardOnlyStream( + CreateCorruptArchive(offset, original, replacement) + ); + using var reader = ReaderFactory.OpenReader(stream, ReaderOptions.ForExternalStream); + + var exception = Assert.ThrowsAny(() => + { + while (reader.MoveToNextEntry()) + { + // Disposal must also terminate when a failed read leaves the decoder between states. + using var entry = reader.OpenEntryStream(); + if (!skipEntry) + { + entry.CopyTo(Stream.Null); + } + } + }); + + Assert.StartsWith("BZip2", exception.Message); + if (!skipEntry) + { + Assert.IsType(exception); + } + } + + [Theory] + [InlineData(240, 0x8d, 0x5a, false)] + [InlineData(11318, 0x35, 0xa8, false)] + [InlineData(41884, 0x12, 0x89, false)] + [InlineData(240, 0x8d, 0x5a, true)] + [InlineData(11318, 0x35, 0xa8, true)] + [InlineData(41884, 0x12, 0x89, true)] + public async Task Zip_BZip2_Reader_Throws_On_Corruption_Async( + int offset, + byte original, + byte replacement, + bool skipEntry + ) + { + using var stream = new AsyncOnlyStream( + new ForwardOnlyStream(CreateCorruptArchive(offset, original, replacement)) + ); + await using var reader = await ReaderFactory.OpenAsyncReader( + stream, + ReaderOptions.ForExternalStream, + cancellationToken: CancellationToken.None + ); + + var exception = await Assert.ThrowsAnyAsync(async () => + { + while (await reader.MoveToNextEntryAsync(CancellationToken.None)) + { +#if LEGACY_DOTNET + using var entry = await reader.OpenEntryStreamAsync(CancellationToken.None); +#else + await using var entry = await reader.OpenEntryStreamAsync(CancellationToken.None); +#endif + if (!skipEntry) + { + await entry.CopyToAsync(Stream.Null, 81920, CancellationToken.None); + } + } + }); + + Assert.StartsWith("BZip2", exception.Message); + if (!skipEntry) + { + Assert.IsType(exception); + } + } + + [Theory] + [InlineData(240, 0x8d, 0x5a)] + [InlineData(11318, 0x35, 0xa8)] + [InlineData(41884, 0x12, 0x89)] + public void Zip_BZip2_Archive_Throws_On_Corruption(int offset, byte original, byte replacement) + { + using var stream = CreateCorruptArchive(offset, original, replacement); + using var archive = ArchiveFactory.OpenArchive(stream, ReaderOptions.ForExternalStream); + + Assert.ThrowsAny(() => + { + foreach (var entry in archive.Entries) + { + if (!entry.IsDirectory) + { + using var entryStream = entry.OpenEntryStream(); + entryStream.CopyTo(Stream.Null); + } + } + }); + } + + [Theory] + [InlineData(240, 0x8d, 0x5a)] + [InlineData(11318, 0x35, 0xa8)] + [InlineData(41884, 0x12, 0x89)] + public async Task Zip_BZip2_Archive_Throws_On_Corruption_Async( + int offset, + byte original, + byte replacement + ) + { + using var stream = new AsyncOnlyStream(CreateCorruptArchive(offset, original, replacement)); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + stream, + ReaderOptions.ForExternalStream, + cancellationToken: CancellationToken.None + ); + + await Assert.ThrowsAnyAsync(async () => + { + await foreach (var entry in archive.EntriesAsync) + { + if (!entry.IsDirectory) + { +#if LEGACY_DOTNET + using var entryStream = await entry.OpenEntryStreamAsync( + CancellationToken.None + ); +#else + await using var entryStream = await entry.OpenEntryStreamAsync( + CancellationToken.None + ); +#endif + await entryStream.CopyToAsync(Stream.Null, 81920, CancellationToken.None); + } + } + }); + } + + private static MemoryStream CreateCorruptArchive(int offset, byte original, byte replacement) + { + // Mutate the existing fixture in memory so each regression differs by exactly one byte. + var bytes = File.ReadAllBytes(Path.Combine(TEST_ARCHIVES_PATH, "Zip.bzip2.dd.zip")); + Assert.Equal(original, bytes[offset]); + bytes[offset] = replacement; + return new MemoryStream(bytes); + } +} From b85ccd28d8585bdbc6b7c37af9acc91677ba9fd8 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 11:20:50 +0000 Subject: [PATCH 08/26] Fix netstandard2.0/net48 build in writable archive key normalisation string.StartsWith(char) is not available on netstandard2.0 or net48, so those targets failed to compile after 990186ac. Co-Authored-By: Claude Opus 5.5 --- src/SharpCompress/Archives/AbstractWritableArchive.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.cs b/src/SharpCompress/Archives/AbstractWritableArchive.cs index 42ff5d5d..f2a7ac87 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.cs @@ -170,7 +170,7 @@ public abstract partial class AbstractWritableArchive } return key.Trim('/'); } - return key.StartsWith('/') ? key.Substring(1) : key; + return key.Length > 0 && key[0] == '/' ? key.Substring(1) : key; } ValueTask IWritableAsyncArchive.RemoveEntryAsync(IArchiveEntry entry) => From 46ba719daa21a9e4826fbc51a52c0da3284c46be Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 11:28:55 +0000 Subject: [PATCH 09/26] Reject negative TAR entry sizes and overflowing seek offsets --- .../Common/Tar/Headers/TarHeader.cs | 13 +- .../Common/Tar/TarHeaderFactory.Async.cs | 4 +- .../Common/Tar/TarHeaderFactory.cs | 6 +- .../Tar/TarSizeValidationTests.cs | 271 ++++++++++++++++++ 4 files changed, 286 insertions(+), 8 deletions(-) create mode 100644 tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs diff --git a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs index 0239b118..27817f96 100644 --- a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs +++ b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs @@ -633,12 +633,19 @@ internal sealed partial class TarHeader private long ReadSize(byte[] buffer) { - if ((buffer[124] & 0x80) == 0x80) // if size in binary + var size = + (buffer[124] & 0x80) == 0x80 // if size in binary + ? BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(0x80)) + : ReadAsciiInt64Base8(buffer, 124, 11); + + // A negative payload size can rewind seekable archives to the same header. + // Validate here so ordinary entries and metadata use the same sync/async guard. + if (size < 0) { - return BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(0x80)); + throw new InvalidFormatException($"Tar entry size {size} cannot be negative."); } - return ReadAsciiInt64Base8(buffer, 124, 11); + return size; } private static byte[] ReadBlock(BinaryReader reader) diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs index c011abd2..ed5a62de 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs @@ -38,8 +38,8 @@ internal static partial class TarHeaderFactory { header.DataStartPosition = stream.Position; - //skip to nearest 512 - stream.Position += PadTo512(header.Size); + // Reject padded sizes or end offsets that would wrap into a backward seek. + stream.Position = checked(stream.Position + PadTo512(header.Size)); } break; case StreamingMode.Streaming: diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs index c95efaef..1628b95a 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs @@ -32,8 +32,8 @@ internal static partial class TarHeaderFactory { header.DataStartPosition = stream.Position; - //skip to nearest 512 - stream.Position += PadTo512(header.Size); + // Reject padded sizes or end offsets that would wrap into a backward seek. + stream.Position = checked(stream.Position + PadTo512(header.Size)); } break; case StreamingMode.Streaming: @@ -64,6 +64,6 @@ internal static partial class TarHeaderFactory { return size; } - return 512 - zeros + size; + return checked(512 - zeros + size); } } diff --git a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs new file mode 100644 index 00000000..93510361 --- /dev/null +++ b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs @@ -0,0 +1,271 @@ +using System; +using System.Buffers.Binary; +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Common.Tar.Headers; +using SharpCompress.IO; +using SharpCompress.Readers; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.Tar; + +public class TarSizeValidationTests +{ + [Theory] + [InlineData(-1L, false)] + [InlineData(-512L, false)] + [InlineData(-1024L, false)] + [InlineData(long.MinValue, false)] + [InlineData(-1L, true)] + [InlineData(-512L, true)] + [InlineData(-1024L, true)] + [InlineData(long.MinValue, true)] + public async Task Tar_Header_Rejects_Negative_Size(long size, bool negativeMarker) + { + using var stream = new MemoryStream(CreateHeader(size, negativeMarker)); + using var reader = new BinaryReader(stream, Encoding.UTF8, leaveOpen: true); + var header = new TarHeader(new ArchiveEncoding()); + + Assert.Throws(() => header.Read(reader)); + + stream.Position = 0; + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + using var asyncReader = new AsyncBinaryReader(asyncStream, leaveOpen: true); + await Assert.ThrowsAsync(async () => + await header.ReadAsync(asyncReader, cancellationToken: CancellationToken.None) + ); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Tar_ArchiveFactory_Rejects_Negative_First_Entry(bool useAsync) + { + // The advisory payload is just one checksum-valid header with size -512. + using var stream = new MemoryStream(CreateHeader(-512)); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await Assert.ThrowsAsync(async () => + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + }); + } + else + { + Assert.Throws(() => + { + using var archive = ArchiveFactory.OpenArchive( + stream, + ReaderOptions.ForExternalStream + ); + }); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(-1L, false)] + [InlineData(-512L, false)] + [InlineData(-1024L, false)] + [InlineData(long.MinValue, false)] + [InlineData(long.MaxValue, false)] + [InlineData(long.MaxValue - 511, false)] + [InlineData(-1L, true)] + [InlineData(-512L, true)] + [InlineData(-1024L, true)] + [InlineData(long.MinValue, true)] + [InlineData(long.MaxValue, true)] + [InlineData(long.MaxValue - 511, true)] + public async Task Tar_Archive_Rejects_Invalid_Entry_Size(long size, bool useAsync) + { + using var stream = CreateArchive(size); + // A valid first entry bypasses format detection so enumeration must validate + // the next header. Only advance twice to keep regressions from looping. + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + await using var entries = archive.EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + Assert.True(await entries.MoveNextAsync()); + await Assert.ThrowsAsync(async () => + await entries.MoveNextAsync() + ); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream, ReaderOptions.ForExternalStream); + using var entries = archive.Entries.GetEnumerator(); + Assert.True(entries.MoveNext()); + Assert.Throws(() => entries.MoveNext()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Tar_Reader_Rejects_Negative_Entry_Size(bool useAsync) + { + using var stream = new ForwardOnlyStream(CreateArchive(-512)); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var reader = await ReaderFactory.OpenAsyncReader( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + await Assert.ThrowsAsync(async () => + await reader.MoveToNextEntryAsync(CancellationToken.None) + ); + } + else + { + using var reader = ReaderFactory.OpenReader(stream, ReaderOptions.ForExternalStream); + Assert.True(reader.MoveToNextEntry()); + Assert.Throws(() => reader.MoveToNextEntry()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(0, false)] + [InlineData(1, false)] + [InlineData(511, false)] + [InlineData(512, false)] + [InlineData(513, false)] + [InlineData(0, true)] + [InlineData(1, true)] + [InlineData(511, true)] + [InlineData(512, true)] + [InlineData(513, true)] + public async Task Tar_Archive_Preserves_Nonnegative_Binary_Sizes(int size, bool useAsync) + { + using var stream = CreateArchive(size); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + await using var entries = archive.EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + Assert.True(await entries.MoveNextAsync()); + Assert.Equal(0, entries.Current.Size); + Assert.True(await entries.MoveNextAsync()); + Assert.Equal(size, entries.Current.Size); + Assert.False(await entries.MoveNextAsync()); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream, ReaderOptions.ForExternalStream); + using var entries = archive.Entries.GetEnumerator(); + Assert.True(entries.MoveNext()); + Assert.Equal(0, entries.Current.Size); + Assert.True(entries.MoveNext()); + Assert.Equal(size, entries.Current.Size); + Assert.False(entries.MoveNext()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(0, false)] + [InlineData(1, false)] + [InlineData(511, false)] + [InlineData(512, false)] + [InlineData(513, false)] + [InlineData(0, true)] + [InlineData(1, true)] + [InlineData(511, true)] + [InlineData(512, true)] + [InlineData(513, true)] + public async Task Tar_Reader_Preserves_Nonnegative_Binary_Sizes(int size, bool useAsync) + { + using var stream = new ForwardOnlyStream(CreateArchive(size)); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var reader = await ReaderFactory.OpenAsyncReader( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + Assert.Equal(0, reader.Entry.Size); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + Assert.Equal(size, reader.Entry.Size); + Assert.False(await reader.MoveToNextEntryAsync(CancellationToken.None)); + } + else + { + using var reader = ReaderFactory.OpenReader(stream, ReaderOptions.ForExternalStream); + Assert.True(reader.MoveToNextEntry()); + Assert.Equal(0, reader.Entry.Size); + Assert.True(reader.MoveToNextEntry()); + Assert.Equal(size, reader.Entry.Size); + Assert.False(reader.MoveToNextEntry()); + } + Assert.True(stream.CanRead); + } + + private static MemoryStream CreateArchive(long size) + { + var stream = new MemoryStream(); + var firstHeader = CreateHeader(0); + stream.Write(firstHeader, 0, firstHeader.Length); + var header = CreateHeader(size); + stream.Write(header, 0, header.Length); + // Invalid sizes need no payload, and must never cause large test allocations. + if (size >= 0 && size <= 1024) + { + stream.SetLength(stream.Length + ((size + 511) / 512 * 512)); + } + stream.SetLength(stream.Length + 1024); + stream.Position = 0; + return stream; + } + + private static byte[] CreateHeader(long size, bool negativeMarker = false) + { + var header = new byte[512]; + header[0] = (byte)'x'; + header[124] = 0x80; + if (negativeMarker) + { + // Cover GNU negative encoding as well as the advisory's 0x80 prefix. + for (var i = 124; i < 128; i++) + { + header[i] = 0xff; + } + } + BinaryPrimitives.WriteInt64BigEndian(header.AsSpan(128, 8), size); + header[156] = (byte)'0'; + Encoding.ASCII.GetBytes("ustar\0" + "00").CopyTo(header, 257); + var checksum = TarHeader.RecalculateChecksum(header); + Encoding + .ASCII.GetBytes(Convert.ToString(checksum, 8).PadLeft(6, '0') + "\0 ") + .CopyTo(header, 148); + return header; + } +} From 30586fdcc5a28631ddb271253b09af98fc68dfb7 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 11:34:11 +0000 Subject: [PATCH 10/26] Validate full-width TAR binary sizes Combine the decoder and padding size bounds from advisory PR #1 with the checked seek offsets and sync/async regression coverage in PR #2. Co-authored-by: 4nonym0us <2701409+4nonym0us@users.noreply.github.com> --- .../Common/Tar/Headers/TarHeader.cs | 26 +++++-- .../Common/Tar/TarHeaderFactory.cs | 9 ++- .../Tar/TarSizeValidationTests.cs | 74 ++++++++++++++++++- 3 files changed, 97 insertions(+), 12 deletions(-) diff --git a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs index 27817f96..559a27bc 100644 --- a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs +++ b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs @@ -37,6 +37,7 @@ internal sealed partial class TarHeader internal IArchiveEncoding ArchiveEncoding { get; } internal const int BLOCK_SIZE = 512; + internal const long MAX_ENTRY_SIZE = long.MaxValue - (BLOCK_SIZE - 1); // Maximum size for long name/link headers to prevent memory exhaustion attacks // This is generous enough for most real-world scenarios (32KB) @@ -631,23 +632,36 @@ internal sealed partial class TarHeader private static EntryType ReadEntryType(byte[] buffer) => (EntryType)buffer[156]; - private long ReadSize(byte[] buffer) + private static long ReadSize(byte[] buffer) { var size = - (buffer[124] & 0x80) == 0x80 // if size in binary - ? BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(0x80)) + (buffer[124] & 0x80) != 0 + ? ReadBase256Size(buffer) : ReadAsciiInt64Base8(buffer, 124, 11); - // A negative payload size can rewind seekable archives to the same header. + // Negative sizes can rewind seekable archives; oversized sizes overflow padding. // Validate here so ordinary entries and metadata use the same sync/async guard. - if (size < 0) + if (size is < 0 or > MAX_ENTRY_SIZE) { - throw new InvalidFormatException($"Tar entry size {size} cannot be negative."); + throw new InvalidFormatException($"Invalid TAR entry size: {size}."); } return size; } + private static long ReadBase256Size(byte[] buffer) + { + // Only the last eight bytes fit in a long. Do not silently truncate the + // remaining value bits, including the sign bits of GNU negative sizes. + var leadingBits = (buffer[124] & 0x7F) | buffer[125] | buffer[126] | buffer[127]; + if (leadingBits != 0) + { + throw new InvalidFormatException("TAR entry size does not fit in 64 bits."); + } + + return BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(128, 8)); + } + private static byte[] ReadBlock(BinaryReader reader) { var buffer = reader.ReadBytes(BLOCK_SIZE); diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs index 1628b95a..e72b23c1 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs @@ -59,11 +59,12 @@ internal static partial class TarHeaderFactory private static long PadTo512(long size) { - var zeros = (int)(size % 512); - if (zeros == 0) + // PAX size overrides bypass ReadSize, so validate the effective size too. + if (size is < 0 or > TarHeader.MAX_ENTRY_SIZE) { - return size; + throw new InvalidFormatException($"Invalid TAR entry size: {size}."); } - return checked(512 - zeros + size); + + return (size + 511) & ~511L; } } diff --git a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs index 93510361..a8e8c6f6 100644 --- a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs +++ b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs @@ -21,11 +21,13 @@ public class TarSizeValidationTests [InlineData(-512L, false)] [InlineData(-1024L, false)] [InlineData(long.MinValue, false)] + [InlineData(long.MaxValue, false)] + [InlineData(TarHeader.MAX_ENTRY_SIZE + 1, false)] [InlineData(-1L, true)] [InlineData(-512L, true)] [InlineData(-1024L, true)] [InlineData(long.MinValue, true)] - public async Task Tar_Header_Rejects_Negative_Size(long size, bool negativeMarker) + public async Task Tar_Header_Rejects_Invalid_Size(long size, bool negativeMarker) { using var stream = new MemoryStream(CreateHeader(size, negativeMarker)); using var reader = new BinaryReader(stream, Encoding.UTF8, leaveOpen: true); @@ -41,6 +43,69 @@ public class TarSizeValidationTests ); } + [Theory] + [InlineData(TarHeader.MAX_ENTRY_SIZE - 1)] + [InlineData(TarHeader.MAX_ENTRY_SIZE)] + public async Task Tar_Header_Accepts_Maximum_Paddable_Size(long size) + { + using var stream = new MemoryStream(CreateHeader(size)); + using var reader = new BinaryReader(stream, Encoding.UTF8, leaveOpen: true); + var header = new TarHeader(new ArchiveEncoding()); + + Assert.True(header.Read(reader)); + Assert.Equal(size, header.Size); + + stream.Position = 0; + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + using var asyncReader = new AsyncBinaryReader(asyncStream, leaveOpen: true); + Assert.True(await header.ReadAsync(asyncReader, cancellationToken: CancellationToken.None)); + Assert.Equal(size, header.Size); + } + + [Theory] + [InlineData(124, false)] + [InlineData(125, false)] + [InlineData(126, false)] + [InlineData(127, false)] + [InlineData(124, true)] + [InlineData(125, true)] + [InlineData(126, true)] + [InlineData(127, true)] + public async Task Tar_ArchiveFactory_Rejects_Binary_Size_With_Ignored_High_Bits( + int offset, + bool useAsync + ) + { + var header = CreateHeader(0); + // Keep the binary marker, but encode a size larger than a long can hold. + header[offset] |= 0x01; + WriteChecksum(header); + using var stream = new MemoryStream(header); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await Assert.ThrowsAsync(async () => + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + }); + } + else + { + Assert.Throws(() => + { + using var archive = ArchiveFactory.OpenArchive( + stream, + ReaderOptions.ForExternalStream + ); + }); + } + Assert.True(stream.CanRead); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -262,10 +327,15 @@ public class TarSizeValidationTests BinaryPrimitives.WriteInt64BigEndian(header.AsSpan(128, 8), size); header[156] = (byte)'0'; Encoding.ASCII.GetBytes("ustar\0" + "00").CopyTo(header, 257); + WriteChecksum(header); + return header; + } + + private static void WriteChecksum(byte[] header) + { var checksum = TarHeader.RecalculateChecksum(header); Encoding .ASCII.GetBytes(Convert.ToString(checksum, 8).PadLeft(6, '0') + "\0 ") .CopyTo(header, 148); - return header; } } From 8ec2188ee04422091a794693f8a86d070f775973 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 11:53:34 +0000 Subject: [PATCH 11/26] Validate 7z header counts before allocation Bound nested metadata counts and coder properties by their backing data, validate file and stream consistency, and buffer decoded headers incrementally instead of reserving attacker-declared output sizes. Add synchronous and asynchronous allocation regressions for GHSA-vwpg-q8vc-g6fg, including external metadata and valid bit-packed empty entries. --- docs/USAGE.md | 2 + .../Common/SevenZip/ArchiveReader.Async.cs | 30 +- .../Common/SevenZip/ArchiveReader.cs | 182 +++++++- src/SharpCompress/Common/SevenZip/CFolder.cs | 9 +- .../Common/SevenZip/DataReader.cs | 22 +- .../SevenZip/SevenZipHeaderAllocationTests.cs | 405 ++++++++++++++++++ 6 files changed, 615 insertions(+), 35 deletions(-) create mode 100644 tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs diff --git a/docs/USAGE.md b/docs/USAGE.md index f17bd9a6..0063c175 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -480,4 +480,6 @@ ARC LZW compressed data is buffered incrementally from the bytes actually presen ZIP Shrink output buffering grows with decoded output rather than the declared uncompressed size. Invalid codes and output-size mismatches are rejected. +7z header counts and coder property lengths are validated against their backing metadata before allocation. File counts are checked against parsed substreams and empty-stream bitmaps, including external metadata. Encoded headers and additional metadata are buffered incrementally as the decoder produces bytes, rather than preallocating the declared unpacked size. + LZMA/LZMA2 dictionary storage starts small and grows with decoded history, up to the supported dictionary size. These safeguards avoid allocations based solely on forged sizes; they do not impose an overall decompressed-output or process-memory limit. diff --git a/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs b/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs index 30c5ad40..f8604921 100644 --- a/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs +++ b/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs @@ -193,6 +193,7 @@ internal sealed partial class ArchiveReader var packIndex = 0; foreach (var folder in folders) { + var unpackSize = GetMetadataSize(folder); var oldDataStartPos = dataStartPos; var myPackSizes = new long[folder._packStreams.Count]; for (var i = 0; i < myPackSizes.Length; i++) @@ -202,7 +203,7 @@ internal sealed partial class ArchiveReader dataStartPos += packSize; } - var outStream = await DecoderStreamHelper + using var outStream = await DecoderStreamHelper .CreateDecoderStreamAsync( _stream, oldDataStartPos, @@ -213,15 +214,29 @@ internal sealed partial class ArchiveReader ) .ConfigureAwait(false); - var unpackSize = checked((int)folder.GetUnpackSize()); - var data = new byte[unpackSize]; - await outStream - .ReadExactAsync(data, 0, data.Length, cancellationToken) - .ConfigureAwait(false); - if (outStream.ReadByte() >= 0) + // Grow only as the decoder actually produces metadata, never + // from the untrusted unpack-size declaration alone. + using var decoded = new PooledMemoryStream(); + var buffer = new byte[8192]; + for (var remaining = unpackSize; remaining > 0; ) + { + var count = Math.Min(buffer.Length, remaining); + await outStream + .ReadExactAsync(buffer, 0, count, cancellationToken) + .ConfigureAwait(false); + await decoded + .WriteAsync(buffer, 0, count, cancellationToken) + .ConfigureAwait(false); + remaining -= count; + } + if ( + await outStream.ReadAsync(buffer, 0, 1, cancellationToken).ConfigureAwait(false) + != 0 + ) { throw new InvalidFormatException("Decoded stream is longer than expected."); } + var data = decoded.ToArray(); dataVector.Add(data); if (folder.UnpackCrcDefined) @@ -319,6 +334,7 @@ internal sealed partial class ArchiveReader } var numFiles = ReadNum(); + ValidateFileCount(numFiles, unpackSizes.Count); db._files = new List(numFiles); for (var i = 0; i < numFiles; i++) { diff --git a/src/SharpCompress/Common/SevenZip/ArchiveReader.cs b/src/SharpCompress/Common/SevenZip/ArchiveReader.cs index 3c9ddee9..5f2b7444 100644 --- a/src/SharpCompress/Common/SevenZip/ArchiveReader.cs +++ b/src/SharpCompress/Common/SevenZip/ArchiveReader.cs @@ -92,6 +92,7 @@ internal partial class ArchiveReader private BitVector ReadBitVector(int length) { + _currentReader.RequireBytes(((long)length + 7) / 8); var bits = new BitVector(length); byte data = 0; @@ -202,9 +203,16 @@ internal partial class ArchiveReader try { var numCoders = ReadNum(); + // CFolder.CheckStructure supports at most 32 coders/bind pairs, but runs + // after parsing. Enforce those limits before allocating their lists. + if (numCoders == 0 || numCoders > CFolder.MaxCoders) + { + throw new InvalidFormatException("Invalid number of 7z coders."); + } + _currentReader.RequireBytes(numCoders); folder._coders = new List(numCoders); - var numInStreams = 0; - var numOutStreams = 0; + long numInStreamsTotal = 0; + long numOutStreamsTotal = 0; for (var i = 0; i < numCoders; i++) { try @@ -241,6 +249,7 @@ internal partial class ArchiveReader if ((mainByte & 0x20) != 0) { var propsSize = ReadNum(); + _currentReader.RequireBytes(propsSize); coder._props = new byte[propsSize]; ReadBytes(coder._props, 0, propsSize); } @@ -250,30 +259,48 @@ internal partial class ArchiveReader throw new NotSupportedException(); } - numInStreams += coder._numInStreams; - numOutStreams += coder._numOutStreams; + if (coder._numInStreams == 0 || coder._numOutStreams == 0) + { + throw new InvalidFormatException("Invalid 7z coder stream count."); + } + numInStreamsTotal += coder._numInStreams; + numOutStreamsTotal += coder._numOutStreams; } finally { } } + if ( + numOutStreamsTotal - 1 > CFolder.MaxBindPairs + || numInStreamsTotal > int.MaxValue + || numInStreamsTotal < numOutStreamsTotal + ) + { + throw new InvalidFormatException("Invalid 7z folder stream counts."); + } + var numInStreams = (int)numInStreamsTotal; + var numOutStreams = (int)numOutStreamsTotal; var numBindPairs = numOutStreams - 1; + _currentReader.RequireBytes(2L * numBindPairs); folder._bindPairs = new List(numBindPairs); for (var i = 0; i < numBindPairs; i++) { var bp = new CBindPair(); bp._inIndex = ReadNum(); bp._outIndex = ReadNum(); + if ( + bp._inIndex >= numInStreams + || bp._outIndex >= numOutStreams + || folder.FindBindPairForInStream(bp._inIndex) >= 0 + || folder.FindBindPairForOutStream(bp._outIndex) >= 0 + ) + { + throw new InvalidFormatException("Invalid 7z bind pair."); + } folder._bindPairs.Add(bp); } - if (numInStreams < numBindPairs) - { - throw new NotSupportedException(); - } - var numPackStreams = numInStreams - numBindPairs; - //folder.PackStreams.Reserve(numPackStreams); if (numPackStreams == 1) { for (var i = 0; i < numInStreams; i++) @@ -292,9 +319,19 @@ internal partial class ArchiveReader } else { + _currentReader.RequireBytes(numPackStreams); + var packStreams = new HashSet(); for (var i = 0; i < numPackStreams; i++) { var num = ReadNum(); + if ( + num >= numInStreams + || folder.FindBindPairForInStream(num) >= 0 + || !packStreams.Add(num) + ) + { + throw new InvalidFormatException("Invalid 7z packed stream index."); + } folder._packStreams.Add(num); } } @@ -304,7 +341,23 @@ internal partial class ArchiveReader private List ReadHashDigests(int count) { + // An all-defined vector has no bitmap, but must have a CRC per item. + // Check it before ReadOptionalBitVector allocates the implicit bitmap. + var reader = _currentReader.Clone(); + if (reader.ReadByte() != 0) + { + reader.RequireBytes(4L * count); + } var defined = ReadOptionalBitVector(count); + long numDefined = 0; + for (var i = 0; i < count; i++) + { + if (defined[i]) + { + numDefined++; + } + } + _currentReader.RequireBytes(4 * numDefined); var digests = new List(count); for (var i = 0; i < count; i++) { @@ -336,6 +389,7 @@ internal partial class ArchiveReader var numPackStreams = ReadNum(); WaitAttribute(BlockType.Size); + _currentReader.RequireBytes(numPackStreams); packSizes = new List(numPackStreams); for (var i = 0; i < numPackStreams; i++) { @@ -382,8 +436,9 @@ internal partial class ArchiveReader { streamSwitch.Set(this, dataVector ?? []); - //folders.Clear(); - //folders.Reserve(numFolders); + // Folder definitions may live in an external metadata stream. + // Validate against the selected stream, not the outer header. + _currentReader.RequireBytes(numFolders); folders = new List(numFolders); var index = 0; for (var i = 0; i < numFolders; i++) @@ -405,6 +460,10 @@ internal partial class ArchiveReader var size = checked((long)ReadNumber()); folder._unpackSizes.Add(size); } + if (!folder.CheckStructure()) + { + throw new InvalidFormatException("Invalid 7z folder structure."); + } } for (; ; ) @@ -476,6 +535,21 @@ internal partial class ArchiveReader } } + // Every substream except the last in each folder needs an explicit + // size. Reject impossible counts before loops or digest allocations. + long numExplicitSizes = 0; + long numStreamsTotal = 0; + foreach (var count in numUnpackStreamsInFolders) + { + numExplicitSizes += Math.Max(0, count - 1); + numStreamsTotal += count; + } + if (numStreamsTotal > int.MaxValue || (numExplicitSizes != 0 && type != BlockType.Size)) + { + throw new InvalidFormatException("Invalid 7z substream count."); + } + _currentReader.RequireBytes(numExplicitSizes); + unpackSizes = new List(folders.Count); for (var i = 0; i < numUnpackStreamsInFolders.Count; i++) { @@ -492,6 +566,10 @@ internal partial class ArchiveReader if (type == BlockType.Size) { var size = checked((long)ReadNumber()); + if (size > folders[i].GetUnpackSize() - sum) + { + throw new InvalidFormatException("Invalid 7z substream size."); + } unpackSizes.Add(size); sum += size; } @@ -620,6 +698,67 @@ internal partial class ArchiveReader finally { } } + private static int GetMetadataSize(CFolder folder) + { + var size = folder.GetUnpackSize(); + if (size < 0 || size > int.MaxValue) + { + throw new InvalidFormatException("7z metadata is too large."); + } + return (int)size; + } + + private void ValidateFileCount(int numFiles, int numStreams) + { + // Nonempty files correspond to already parsed substreams. Empty files + // require one bit each in kEmptyStream, not one byte per file. Preflight + // that property before allocating CFileItem objects or implicit vectors. + var reader = _currentReader.Clone(); + var numEmptyStreams = 0; + var hasEmptyStreams = false; + for (; ; ) + { + var type = reader.ReadNumber(); + if (type == (ulong)BlockType.End) + { + break; + } + var size = reader.ReadNumber(); + if (size > (ulong)reader.Remaining) + { + throw new IncompleteArchiveException("Unexpected end of stream."); + } + var start = reader.Offset; + if (type == (ulong)BlockType.EmptyStream) + { + var bitmapSize = ((long)numFiles + 7) / 8; + if (hasEmptyStreams || size < (ulong)bitmapSize) + { + throw new InvalidFormatException("Invalid 7z empty stream bitmap."); + } + hasEmptyStreams = true; + byte bits = 0; + for (var i = 0; i < numFiles; i++) + { + if ((i & 7) == 0) + { + bits = reader.ReadByte(); + } + if ((bits & 0x80) != 0) + { + numEmptyStreams++; + } + bits <<= 1; + } + } + reader.SkipData((long)size - (reader.Offset - start)); + } + if ((long)numStreams + numEmptyStreams != numFiles) + { + throw new InvalidFormatException("7z file count does not match the stream count."); + } + } + private List ReadAndDecodePackedStreams(long baseOffset, IPasswordProvider pass) { try @@ -641,6 +780,7 @@ internal partial class ArchiveReader var packIndex = 0; foreach (var folder in folders) { + var unpackSize = GetMetadataSize(folder); var oldDataStartPos = dataStartPos; var myPackSizes = new long[folder._packStreams.Count]; for (var i = 0; i < myPackSizes.Length; i++) @@ -650,7 +790,7 @@ internal partial class ArchiveReader dataStartPos += packSize; } - var outStream = DecoderStreamHelper.CreateDecoderStream( + using var outStream = DecoderStreamHelper.CreateDecoderStream( _stream, oldDataStartPos, myPackSizes, @@ -658,13 +798,22 @@ internal partial class ArchiveReader pass ); - var unpackSize = checked((int)folder.GetUnpackSize()); - var data = new byte[unpackSize]; - outStream.ReadExact(data, 0, data.Length); + // Do not reserve the declared unpack size: malformed compressed + // metadata can advertise GiB of output without producing a byte. + using var decoded = new PooledMemoryStream(); + var buffer = new byte[8192]; + for (var remaining = unpackSize; remaining > 0; ) + { + var count = Math.Min(buffer.Length, remaining); + outStream.ReadExact(buffer, 0, count); + decoded.Write(buffer, 0, count); + remaining -= count; + } if (outStream.ReadByte() >= 0) { throw new InvalidFormatException("Decoded stream is longer than expected."); } + var data = decoded.ToArray(); dataVector.Add(data); if (folder.UnpackCrcDefined) @@ -756,6 +905,7 @@ internal partial class ArchiveReader } var numFiles = ReadNum(); + ValidateFileCount(numFiles, unpackSizes.Count); db._files = new List(numFiles); for (var i = 0; i < numFiles; i++) { diff --git a/src/SharpCompress/Common/SevenZip/CFolder.cs b/src/SharpCompress/Common/SevenZip/CFolder.cs index 683c27ca..50ef871e 100644 --- a/src/SharpCompress/Common/SevenZip/CFolder.cs +++ b/src/SharpCompress/Common/SevenZip/CFolder.cs @@ -6,6 +6,9 @@ namespace SharpCompress.Common.SevenZip; internal class CFolder { + internal const int MaxCoders = 32; + internal const int MaxBindPairs = 32; + internal List _coders = new(); internal List _bindPairs = new(); internal List _packStreams = new(); @@ -98,11 +101,9 @@ internal class CFolder public bool CheckStructure() { - const int kNumCodersMax = 32; // don't change it - const int kMaskSize = 32; // it must be >= kNumCodersMax - const int kNumBindsMax = 32; + const int kMaskSize = 32; // it must be >= MaxCoders - if (_coders.Count > kNumCodersMax || _bindPairs.Count > kNumBindsMax) + if (_coders.Count > MaxCoders || _bindPairs.Count > MaxBindPairs) { return false; } diff --git a/src/SharpCompress/Common/SevenZip/DataReader.cs b/src/SharpCompress/Common/SevenZip/DataReader.cs index 8788192b..85d59be5 100644 --- a/src/SharpCompress/Common/SevenZip/DataReader.cs +++ b/src/SharpCompress/Common/SevenZip/DataReader.cs @@ -45,6 +45,18 @@ internal class DataReader public int Offset { get; private set; } + public int Remaining => _ending - Offset; + + public DataReader Clone() => new(_buffer, Offset, Remaining); + + public void RequireBytes(long count) + { + if (count < 0 || count > Remaining) + { + throw new IncompleteArchiveException("Unexpected end of stream."); + } + } + public byte ReadByte() { if (Offset >= _ending) @@ -57,10 +69,7 @@ internal class DataReader public void ReadBytes(byte[] buffer, int offset, int length) { - if (length > _ending - Offset) - { - throw new IncompleteArchiveException("Unexpected end of stream."); - } + RequireBytes(length); while (length-- > 0) { @@ -70,10 +79,7 @@ internal class DataReader public void SkipData(long size) { - if (size > _ending - Offset) - { - throw new IncompleteArchiveException("Unexpected end of stream."); - } + RequireBytes(size); Offset += (int)size; } diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs new file mode 100644 index 00000000..18db66b5 --- /dev/null +++ b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs @@ -0,0 +1,405 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Common.SevenZip; +using SharpCompress.Compressors.LZMA; +using SharpCompress.Crypto; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.SevenZip; + +[CollectionDefinition(nameof(SevenZipHeaderAllocationTests), DisableParallelization = true)] +public class SevenZipHeaderAllocationCollection { } + +[Collection(nameof(SevenZipHeaderAllocationTests))] +public class SevenZipHeaderAllocationTests +{ + public static IEnumerable MalformedHeaders() + { + var count = Number(100_000); + yield return ["folders", Join([1, 4, 7, 11], count, [0])]; + yield return ["files", Join([1, 5], count, [0])]; + yield return ["coders", Join([1, 4, 7, 11, 1, 0], count)]; + yield return ["properties", Join([1, 4, 7, 11, 1, 0, 1, 0x21, 0x21], count)]; + yield return ["pack streams", Join([1, 4, 6, 0], count, [9, 0])]; + yield return ["input streams", Join([1, 4, 7, 11, 1, 0, 1, 0x11, 0], count, [1])]; + yield return ["output streams", Join([1, 4, 7, 11, 1, 0, 1, 0x11, 0, 1], count)]; + yield return ["zero coders", new byte[] { 1, 4, 7, 11, 1, 0, 0 }]; + yield return ["zero input streams", new byte[] { 1, 4, 7, 11, 1, 0, 1, 0x11, 0, 0, 1 }]; + yield return ["zero output streams", new byte[] { 1, 4, 7, 11, 1, 0, 1, 0x11, 0, 1, 0 }]; + yield return ["truncated empty bitmap", Join([1, 5], count, [14, 1, 0x80, 0])]; + // One valid Copy folder followed by forged per-folder substream counts. + byte[] folder = [1, 4, 7, 11, 1, 0, 1, 1, 0, 12, 0, 0, 8, 13]; + yield return ["substream CRCs", Join(folder, count, [10, 1])]; + yield return ["substream sizes", Join(folder, count, [9, 0])]; + yield return ["implicit substream CRCs", Join(folder, count, [0])]; + yield return ["substream size exceeds folder", Join(folder, [2, 9, 1, 0])]; + yield return ["invalid bind index", new byte[] { 1, 4, 7, 11, 1, 0, 2, 1, 0, 1, 0, 2, 0 }]; + yield return + [ + "invalid pack index", + new byte[] { 1, 4, 7, 11, 1, 0, 1, 0x11, 0, 2, 1, 0, 2 }, + ]; + yield return + [ + "duplicate pack index", + new byte[] { 1, 4, 7, 11, 1, 0, 1, 0x11, 0, 2, 1, 0, 0 }, + ]; + yield return + [ + "oversized decoded metadata", + MetadataHeader(true, (ulong)int.MaxValue + 1, 0, [1, 0]), + ]; + yield return + [ + "overflowing inputs", + Join( + [1, 4, 7, 11, 1, 0, 2, 0x11, 0], + Number(int.MaxValue), + [1, 0x11, 0], + Number(int.MaxValue), + [1] + ), + ]; + } + + [Theory] + [MemberData(nameof(MalformedHeaders))] + public void SevenZipArchive_InvalidHeaderCounts_RejectBeforeAllocation( + string name, + byte[] header + ) + { + using var stream = CreateArchive(header); + using var archive = ArchiveFactory.OpenArchive(stream); +#if !LEGACY_DOTNET + var before = GC.GetAllocatedBytesForCurrentThread(); +#endif + var exception = Record.Exception(() => archive.Entries.ToList()); + Assert.True( + exception is InvalidFormatException or IncompleteArchiveException, + $"{name}: {exception}" + ); +#if !LEGACY_DOTNET + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + Assert.True(allocated < 512 * 1024, $"{name}: allocated {allocated} bytes."); +#endif + Assert.True(stream.CanRead); + } + + [Theory] + [MemberData(nameof(MalformedHeaders))] + public async Task SevenZipArchive_InvalidHeaderCounts_AsyncRejectBeforeAllocation( + string name, + byte[] header + ) + { + using var stream = CreateArchive(header); + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + cancellationToken: CancellationToken.None + ); +#if !LEGACY_DOTNET + var before = GC.GetTotalAllocatedBytes(precise: true); +#endif + var exception = await Record.ExceptionAsync(async () => + { + await foreach ( + var entry in archive.EntriesAsync.WithCancellation(CancellationToken.None) + ) { } + }); + Assert.True( + exception is InvalidFormatException or IncompleteArchiveException, + $"{name}: {exception}" + ); +#if !LEGACY_DOTNET + var allocated = GC.GetTotalAllocatedBytes(precise: true) - before; + Assert.True(allocated < 512 * 1024, $"{name}: allocated {allocated} bytes."); +#endif + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void SevenZipArchive_TruncatedDecodedMetadata_DoesNotReserveUnpackSize( + bool encodedHeader + ) + { + // Copy produces only the bytes present, despite advertising 16 MiB. + var header = MetadataHeader(encodedHeader, 16 * 1024 * 1024, 1, [1, 0]); + using var stream = CreateArchive(header, [0]); + using var archive = ArchiveFactory.OpenArchive(stream); +#if !LEGACY_DOTNET + var before = GC.GetAllocatedBytesForCurrentThread(); +#endif + Assert.Throws(() => archive.Entries.ToList()); +#if !LEGACY_DOTNET + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + Assert.True(allocated < 512 * 1024, $"Allocated {allocated} bytes."); +#endif + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZipArchive_TruncatedDecodedMetadata_AsyncRejects(bool encodedHeader) + { + var header = MetadataHeader(encodedHeader, 16 * 1024 * 1024, 1, [1, 0]); + using var stream = CreateArchive(header, [0]); + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + cancellationToken: CancellationToken.None + ); +#if !LEGACY_DOTNET + var before = GC.GetTotalAllocatedBytes(precise: true); +#endif + await Assert.ThrowsAsync(async () => + { + await foreach ( + var entry in archive.EntriesAsync.WithCancellation(CancellationToken.None) + ) { } + }); +#if !LEGACY_DOTNET + var allocated = GC.GetTotalAllocatedBytes(precise: true) - before; + Assert.True(allocated < 512 * 1024, $"Allocated {allocated} bytes."); +#endif + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZipArchive_ReportedLzma2MetadataPayload_Rejects(bool async) + { + // The report puts the first packed byte at the start of the next header. + var header = MetadataHeader(false, 16 * 1024 * 1024, 1, [0x21, 0x21, 1, 0x18]); + using var stream = CreateArchive(header); + if (async) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + cancellationToken: CancellationToken.None + ); +#if !LEGACY_DOTNET + var before = GC.GetTotalAllocatedBytes(precise: true); +#endif + await Assert.ThrowsAsync(async () => + { + await foreach ( + var entry in archive.EntriesAsync.WithCancellation(CancellationToken.None) + ) { } + }); +#if !LEGACY_DOTNET + var allocated = GC.GetTotalAllocatedBytes(precise: true) - before; + Assert.True(allocated < 512 * 1024, $"Allocated {allocated} bytes."); +#endif + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream); +#if !LEGACY_DOTNET + var before = GC.GetAllocatedBytesForCurrentThread(); +#endif + Assert.Throws(() => archive.Entries.ToList()); +#if !LEGACY_DOTNET + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + Assert.True(allocated < 512 * 1024, $"Allocated {allocated} bytes."); +#endif + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZipArchive_BitPackedEmptyFiles_AreValid(bool async) + { + // Nine nameless directories need only two bitmap bytes. A byte-per-file + // bound on the remaining header would incorrectly reject this archive. + using var stream = CreateArchive([1, 5, 9, 14, 2, 0xFF, 0x80, 0, 0]); + if (async) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + stream, + cancellationToken: CancellationToken.None + ); + var count = 0; + await foreach ( + var entry in archive.EntriesAsync.WithCancellation(CancellationToken.None) + ) + { + Assert.True(entry.IsDirectory); + count++; + } + Assert.Equal(9, count); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream); + Assert.Equal(9, archive.Entries.Count()); + Assert.All(archive.Entries, entry => Assert.True(entry.IsDirectory)); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZipArchive_ExternalFolderCount_UsesExternalBuffer(bool async) + { + // Decode one byte of additional metadata, then reference it as the + // folder-definition buffer. The inline header has ample padding. + var header = Join(MetadataHeader(false, 1, 1, [1, 0]), [4, 7, 11, 2, 1, 0], new byte[128]); + using var stream = CreateArchive(header, [0]); + if (async) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + cancellationToken: CancellationToken.None + ); + await Assert.ThrowsAsync(async () => + { + await foreach ( + var entry in archive.EntriesAsync.WithCancellation(CancellationToken.None) + ) { } + }); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream); + Assert.Throws(() => archive.Entries.ToList()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZipArchive_ExternalFolderDefinition_IsValid(bool async) + { + // Additional metadata contains a Copy folder. Its unpack size remains + // inline; the file's packed byte follows the external folder definition. + var header = Join( + MetadataHeader(false, 3, 3, [1, 0]), + [4, 6, 3, 1, 9, 1, 0, 7, 11, 1, 1, 0, 12, 1, 0, 8, 0, 0, 5, 1, 0, 0] + ); + using var stream = CreateArchive(header, [1, 1, 0, 42]); + if (async) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + cancellationToken: CancellationToken.None + ); + var count = 0; + await foreach ( + var entry in archive.EntriesAsync.WithCancellation(CancellationToken.None) + ) + { + using var entryStream = await entry.OpenEntryStreamAsync(CancellationToken.None); + var data = new byte[1]; + Assert.Equal(1, await entryStream.ReadAsync(data, 0, 1, CancellationToken.None)); + Assert.Equal(42, data[0]); + count++; + } + Assert.Equal(1, count); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream); + using var entryStream = archive.Entries.Single().OpenEntryStream(); + Assert.Equal(42, entryStream.ReadByte()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZipArchive_DecodedMetadataSpanningBuffers_IsValid(bool async) + { + // An unknown archive property forces decoding across several buffers. + var decoded = Join([1, 2, 26], Number(20_000), new byte[20_000], [0, 0]); + var header = MetadataHeader(true, (ulong)decoded.Length, (ulong)decoded.Length, [1, 0]); + using var stream = CreateArchive(header, decoded); + if (async) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + cancellationToken: CancellationToken.None + ); + await foreach ( + var entry in archive.EntriesAsync.WithCancellation(CancellationToken.None) + ) + { + Assert.Fail("Expected an empty archive."); + } + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream); + Assert.Empty(archive.Entries); + } + Assert.True(stream.CanRead); + } + + private static byte[] MetadataHeader( + bool encoded, + ulong unpackSize, + ulong packSize, + byte[] coder + ) => + Join( + encoded ? [23] : [1, 3], + [6, 0, 1, 9], + Number(packSize), + [0, 7, 11, 1, 0, 1], + coder, + [12], + Number(unpackSize), + [0, 0] + ); + + private static byte[] Number(ulong value) + { + using var stream = new MemoryStream(); + stream.WriteEncodedUInt64(value); + return stream.ToArray(); + } + + private static byte[] Join(params byte[][] parts) => parts.SelectMany(part => part).ToArray(); + + private static MemoryStream CreateArchive(byte[] header, byte[]? packedData = null) + { + packedData ??= []; + var crc = Crc32Stream.Compute( + Crc32Stream.DEFAULT_POLYNOMIAL, + Crc32Stream.DEFAULT_SEED, + header + ); + var stream = new MemoryStream(); + SevenZipSignatureHeaderWriter.WriteFinal( + stream, + (ulong)packedData.Length, + (ulong)header.Length, + crc + ); + stream.Write(packedData, 0, packedData.Length); + stream.Write(header, 0, header.Length); + stream.Position = 0; + return stream; + } +} From cecd8543a44438e256c914c5ce714f2c3ee75818 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 13:23:53 +0000 Subject: [PATCH 12/26] Bound decompression dictionaries with static size limits --- docs/API.md | 33 ++ docs/USAGE.md | 12 + .../Common/DictionarySizeLimits.cs | 80 ++++ .../Compressors/LZMA/LzmaDecoder.cs | 2 + .../Compressors/LZMA/LzmaStream.cs | 3 + .../Compressors/PPMd/H/SubAllocator.cs | 2 + .../Compressors/PPMd/I1/Allocator.cs | 5 + .../Compressors/Rar/UnpackV1/Unpack.cs | 1 + .../Rar/UnpackV2017/Unpack.unpack_cpp.cs | 12 +- .../Compressors/ZStandard/Decompressor.cs | 16 + .../ZStandard/Unsafe/ZstdDecompress.cs | 20 +- .../Rar/RarDictionarySizeLimitTests.cs | 167 ++++++++ .../Streams/DictionarySizeLimitTests.cs | 389 ++++++++++++++++++ .../Streams/LzmaDictionaryAllocationTests.cs | 5 + 14 files changed, 736 insertions(+), 11 deletions(-) create mode 100644 src/SharpCompress/Common/DictionarySizeLimits.cs create mode 100644 tests/SharpCompress.Test/Rar/RarDictionarySizeLimitTests.cs create mode 100644 tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs diff --git a/docs/API.md b/docs/API.md index c8e2541f..7cf54ba3 100644 --- a/docs/API.md +++ b/docs/API.md @@ -327,6 +327,39 @@ await writer.WriteAllAsync("C:\\source", "*", SearchOption.AllDirectories, cance ## Common Options +### Decompression dictionary limits + +`SharpCompress.Common.DictionarySizeLimits` contains process-wide static size limits +in bytes. Set them once at application startup, not around individual requests: + +```csharp +// Opt in only when trusted archives require larger dictionaries. +DictionarySizeLimits.Rar5 = 256L << 20; +``` + +| Static property | Default maximum | +| --- | --- | +| `Rar4` | 4 MiB (the RAR4 format maximum) | +| `Rar5` | 64 MiB | +| `Lzma` (raw LZMA, LZip, ZIP, 7z) | 64 MiB | +| `Lzma2` (XZ, 7z) | 64 MiB | +| `Ppmd` (ZIP, 7z, RAR models) | 64 MiB | +| `ZStandard` (streaming windows) | 128 MiB | + +Limits are read directly by the shared codecs before dictionary/model allocation, +so Archive, Reader, direct decoder, sync, and async paths use the same settings. +No reader options or decoder overloads are needed. Settings require positive values; +invalid assignments throw `ArgumentOutOfRangeException` without changing the setting. +Oversized dictionaries throw `InvalidFormatException`; Zstandard reports +`ZstdException` with `ZSTD_error_frameParameter_windowTooLarge`. Zstandard's minimum +configurable window limit is 1 KiB. Format and engine restrictions still apply. + +These are dictionary/window or model limits, not total-memory, output-size, or +concurrency limits. Pool rounding and allocator overhead can increase actual memory. +Shared PPMd allocators also apply the model limit during compression. Fixed/bounded +codecs retain their format-defined bounds. Custom providers must enforce their own +limits. Zstandard's one-shot `Unwrap` APIs retain their existing output-size bounds. + ### ReaderOptions Use preset properties and fluent helpers for common configurations: diff --git a/docs/USAGE.md b/docs/USAGE.md index f17bd9a6..6cf47ad4 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -481,3 +481,15 @@ ARC LZW compressed data is buffered incrementally from the bytes actually presen ZIP Shrink output buffering grows with decoded output rather than the declared uncompressed size. Invalid codes and output-size mismatches are rejected. LZMA/LZMA2 dictionary storage starts small and grows with decoded history, up to the supported dictionary size. These safeguards avoid allocations based solely on forged sizes; they do not impose an overall decompressed-output or process-memory limit. + +Shared codecs also check `SharpCompress.Common.DictionarySizeLimits` before +allocating dictionaries/windows or PPMd models. Defaults are 4 MiB for RAR4, +64 MiB for RAR5, LZMA (including LZip), LZMA2 (including XZ), and PPMd, and +128 MiB for Zstandard streaming windows. A tiny RAR5 entry therefore cannot rent +a 256 MiB or 1 GiB window under the default settings. + +These settings are **process-wide**, not per-reader options. Configure them once +at startup if trusted input requires larger dictionaries, for example +`DictionarySizeLimits.Rar5 = 256L << 20;`. Do not temporarily raise them around +untrusted requests. Bound extraction output and concurrency separately; dictionary +limits are not a total-memory budget. See the [API reference](API.md#decompression-dictionary-limits). diff --git a/src/SharpCompress/Common/DictionarySizeLimits.cs b/src/SharpCompress/Common/DictionarySizeLimits.cs new file mode 100644 index 00000000..1ea21d62 --- /dev/null +++ b/src/SharpCompress/Common/DictionarySizeLimits.cs @@ -0,0 +1,80 @@ +using System; +using System.Threading; + +namespace SharpCompress.Common; + +/// +/// Process-wide dictionary/window and PPMd model size limits, in bytes. +/// Configure once at application startup, not for individual extraction requests. +/// These limits do not bound total memory, allocator overhead, or decompressed output. +/// +public static class DictionarySizeLimits +{ + private static long rar4 = 4L << 20; + private static long rar5 = 64L << 20; + private static long lzma = 64L << 20; + private static long lzma2 = 64L << 20; + private static long ppmd = 64L << 20; + private static long zStandard = 128L << 20; + + /// RAR4 window limit. Defaults to the format maximum of 4 MiB. + public static long Rar4 + { + get => Interlocked.Read(ref rar4); + set => SetMaximum(ref rar4, value, nameof(Rar4)); + } + + /// RAR5 window limit. Defaults to 64 MiB. + public static long Rar5 + { + get => Interlocked.Read(ref rar5); + set => SetMaximum(ref rar5, value, nameof(Rar5)); + } + + /// LZMA dictionary limit, including LZip, ZIP, and 7z. Defaults to 64 MiB. + public static long Lzma + { + get => Interlocked.Read(ref lzma); + set => SetMaximum(ref lzma, value, nameof(Lzma)); + } + + /// LZMA2 dictionary limit, including XZ and 7z. Defaults to 64 MiB. + public static long Lzma2 + { + get => Interlocked.Read(ref lzma2); + set => SetMaximum(ref lzma2, value, nameof(Lzma2)); + } + + /// PPMd model limit, including ZIP, 7z, and RAR models. Defaults to 64 MiB. + public static long Ppmd + { + get => Interlocked.Read(ref ppmd); + set => SetMaximum(ref ppmd, value, nameof(Ppmd)); + } + + /// Zstandard streaming window limit. Defaults to 128 MiB. + public static long ZStandard + { + get => Interlocked.Read(ref zStandard); + set => SetMaximum(ref zStandard, value, nameof(ZStandard)); + } + + private static void SetMaximum(ref long field, long value, string name) + { + if (value <= 0) + { + throw new ArgumentOutOfRangeException(name, "Dictionary size limits must be positive."); + } + Interlocked.Exchange(ref field, value); + } + + internal static void Validate(long size, long maximum, string format) + { + if (size <= 0 || size > maximum) + { + throw new InvalidFormatException( + $"{format} dictionary size {size} is invalid or exceeds the maximum of {maximum} bytes." + ); + } + } +} diff --git a/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs b/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs index 02fd16d7..888af50a 100644 --- a/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs +++ b/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics.CodeAnalysis; using System.IO; using System.Threading.Tasks; +using SharpCompress.Common; using SharpCompress.Compressors.LZMA.LZ; using SharpCompress.Compressors.LZMA.RangeCoder; @@ -229,6 +230,7 @@ public partial class Decoder : ICoder, ISetDecoderProperties, IDisposable } _outWindow = new OutWindow(); var blockSize = Math.Max(_dictionarySize, (1 << 12)); + DictionarySizeLimits.Validate(blockSize, DictionarySizeLimits.Lzma, "LZMA"); _outWindow.Create(blockSize); } diff --git a/src/SharpCompress/Compressors/LZMA/LzmaStream.cs b/src/SharpCompress/Compressors/LZMA/LzmaStream.cs index 361378a6..09f4af31 100644 --- a/src/SharpCompress/Compressors/LZMA/LzmaStream.cs +++ b/src/SharpCompress/Compressors/LZMA/LzmaStream.cs @@ -4,6 +4,7 @@ using System.Buffers.Binary; using System.IO; using System.Threading; using System.Threading.Tasks; +using SharpCompress.Common; using SharpCompress.Compressors.LZMA.LZ; using SharpCompress.IO; @@ -54,6 +55,7 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable if (!isLzma2) { _dictionarySize = BinaryPrimitives.ReadInt32LittleEndian(properties.AsSpan(1)); + DictionarySizeLimits.Validate(_dictionarySize, DictionarySizeLimits.Lzma, "LZMA"); _decoder = new Decoder(); _decoder.SetDecoderProperties(properties); Properties = properties; @@ -72,6 +74,7 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable } _dictionarySize = 2 | (properties[0] & 1); _dictionarySize <<= (properties[0] >> 1) + 11; + DictionarySizeLimits.Validate(_dictionarySize, DictionarySizeLimits.Lzma2, "LZMA2"); Properties = new byte[1]; _availableBytes = 0; diff --git a/src/SharpCompress/Compressors/PPMd/H/SubAllocator.cs b/src/SharpCompress/Compressors/PPMd/H/SubAllocator.cs index df6dfa60..8927ff50 100644 --- a/src/SharpCompress/Compressors/PPMd/H/SubAllocator.cs +++ b/src/SharpCompress/Compressors/PPMd/H/SubAllocator.cs @@ -3,6 +3,7 @@ using System; using System.Buffers; using System.Text; +using SharpCompress.Common; namespace SharpCompress.Compressors.PPMd.H; @@ -155,6 +156,7 @@ internal class SubAllocator : IDisposable public virtual bool StartSubAllocator(int saSize) { + DictionarySizeLimits.Validate(saSize, DictionarySizeLimits.Ppmd, "PPMd"); var t = saSize; if (_subAllocatorSize == t) { diff --git a/src/SharpCompress/Compressors/PPMd/I1/Allocator.cs b/src/SharpCompress/Compressors/PPMd/I1/Allocator.cs index c4d8aa4b..0c8846d0 100644 --- a/src/SharpCompress/Compressors/PPMd/I1/Allocator.cs +++ b/src/SharpCompress/Compressors/PPMd/I1/Allocator.cs @@ -1,5 +1,7 @@ #nullable disable +using SharpCompress.Common; + namespace SharpCompress.Compressors.PPMd.I1; /// Allocate a single, large array and then provide sections of this array to callers. Callers are provided with @@ -128,6 +130,9 @@ internal class Allocator /// public void Start(int allocatorSize) { + // PpmdProperties starts this allocator while parsing ZIP properties, so + // enforce the limit here, before any attacker-selected allocation. + DictionarySizeLimits.Validate(allocatorSize, DictionarySizeLimits.Ppmd, "PPMd"); var size = (uint)allocatorSize; if (_allocatorSize != size) { diff --git a/src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.cs b/src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.cs index 0d4c2175..a7bd2887 100644 --- a/src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.cs +++ b/src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.cs @@ -130,6 +130,7 @@ internal sealed partial class Unpack : BitInput, IRarUnpack private void Init() { + DictionarySizeLimits.Validate(PackDef.MAXWINSIZE, DictionarySizeLimits.Rar4, "RAR4"); if (this.window is null) { this.window = ArrayPool.Shared.Rent(PackDef.MAXWINSIZE); diff --git a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs index 72c808f9..c094093f 100644 --- a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs +++ b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs @@ -99,14 +99,22 @@ internal sealed partial class Unpack : BitInput WinSize = MinAllocSize; } + // Validate before renting or reusing a solid window. The RAR5 header can + // request a huge dictionary even when there are no compressed bytes. + DictionarySizeLimits.Validate( + WinSize, + fileHeader.IsRar5 ? DictionarySizeLimits.Rar5 : DictionarySizeLimits.Rar4, + fileHeader.IsRar5 ? "RAR5" : "RAR4" + ); + if (WinSize <= MaxWinSize) // Use the already allocated window. { return; } - if ((WinSize >> 16) > 0x10000) // Window size must not exceed 4 GB. + if (WinSize > int.MaxValue) { - return; + throw new InvalidFormatException("RAR window exceeds the supported array size."); } // Archiving code guarantees that window size does not grow in the same diff --git a/src/SharpCompress/Compressors/ZStandard/Decompressor.cs b/src/SharpCompress/Compressors/ZStandard/Decompressor.cs index 06da3b8f..4a0b0e8f 100644 --- a/src/SharpCompress/Compressors/ZStandard/Decompressor.cs +++ b/src/SharpCompress/Compressors/ZStandard/Decompressor.cs @@ -1,4 +1,5 @@ using System; +using SharpCompress.Common; using SharpCompress.Compressors.ZStandard.Unsafe; namespace SharpCompress.Compressors.ZStandard; @@ -10,6 +11,21 @@ public unsafe class Decompressor : IDisposable public Decompressor() { handle = SafeDctxHandle.Create(); + try + { + using var dctx = handle.Acquire(); + Unsafe + .Methods.ZSTD_DCtx_setMaxWindowSize( + dctx, + checked((nuint)DictionarySizeLimits.ZStandard) + ) + .EnsureZstdSuccess(); + } + catch + { + handle.Dispose(); + throw; + } } public void SetParameter(ZSTD_dParameter parameter, int value) diff --git a/src/SharpCompress/Compressors/ZStandard/Unsafe/ZstdDecompress.cs b/src/SharpCompress/Compressors/ZStandard/Unsafe/ZstdDecompress.cs index ab954fb6..d1b4be8f 100644 --- a/src/SharpCompress/Compressors/ZStandard/Unsafe/ZstdDecompress.cs +++ b/src/SharpCompress/Compressors/ZStandard/Unsafe/ZstdDecompress.cs @@ -3201,6 +3201,17 @@ public static unsafe partial class Methods } } + // Apply the limit before the single-pass shortcut too, so it + // does not depend on the caller's output buffer size. + zds->fParams.windowSize = + zds->fParams.windowSize > 1U << 10 ? zds->fParams.windowSize : 1U << 10; + if (zds->fParams.windowSize > zds->maxWindowSize) + { + return unchecked( + (nuint)(-(int)ZSTD_ErrorCode.ZSTD_error_frameParameter_windowTooLarge) + ); + } + if ( zds->fParams.frameContentSize != unchecked(0UL - 1) && zds->fParams.frameType != ZSTD_frameType_e.ZSTD_skippableFrame @@ -3282,15 +3293,6 @@ public static unsafe partial class Methods zds->stage = ZSTD_dStage.ZSTDds_decodeBlockHeader; } - zds->fParams.windowSize = - zds->fParams.windowSize > 1U << 10 ? zds->fParams.windowSize : 1U << 10; - if (zds->fParams.windowSize > zds->maxWindowSize) - { - return unchecked( - (nuint)(-(int)ZSTD_ErrorCode.ZSTD_error_frameParameter_windowTooLarge) - ); - } - if (zds->maxBlockSizeParam != 0) { zds->fParams.blockSizeMax = diff --git a/tests/SharpCompress.Test/Rar/RarDictionarySizeLimitTests.cs b/tests/SharpCompress.Test/Rar/RarDictionarySizeLimitTests.cs new file mode 100644 index 00000000..6c85b8db --- /dev/null +++ b/tests/SharpCompress.Test/Rar/RarDictionarySizeLimitTests.cs @@ -0,0 +1,167 @@ +using System; +using System.Buffers.Binary; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Compressors.Xz; +using SharpCompress.Readers; +using SharpCompress.Test.Streams; +using Xunit; + +namespace SharpCompress.Test.Rar; + +[Collection(DictionarySizeLimitCollection.Name)] +public class RarDictionarySizeLimitTests +{ + // The advisory's complete 72-byte archives have valid header CRCs, no packed + // data, and just 45,056 bytes of declared output. Neither needs a large window. + private const string Rar256MiB = + "526172211a070100b28180510c01050800070101c496868000c63c6d612a02030b8080000480e00220c809b1cf8059000c6578652f746573742e6578650a03020058a2ce32ffc101"; + private const string Rar1GiB = + "526172211a070100b28180510c01050800070101c4968680000fd7ea442a02030b8080000480e00220c809b1cf8069000c6578652f746573742e6578650a03020058a2ce32ffc101"; + + public static IEnumerable Payloads() + { + foreach (var hex in new[] { Rar256MiB, Rar1GiB }) + { + foreach (var archiveApi in new[] { false, true }) + { + foreach (var useAsync in new[] { false, true }) + { + yield return new object[] { hex, archiveApi, useAsync }; + } + } + } + } + + [Theory] + [MemberData(nameof(Payloads))] + public async Task Rar5_Huge_Window_Is_Rejected_Before_Decoding( + string hex, + bool archiveApi, + bool useAsync + ) + { + var bytes = DecodeHex(hex); + Assert.Equal(72, bytes.Length); + using var input = new MemoryStream(bytes); + var options = ReaderOptions.ForExternalStream; + InvalidFormatException exception; + if (archiveApi && useAsync) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + input, + options, + CancellationToken.None + ); + var entry = await archive.EntriesAsync.FirstAsync(); + Assert.Equal(45056, entry.Size); + Assert.Equal(0, entry.CompressedSize); + exception = await Assert.ThrowsAsync(async () => + { + using var stream = await entry.OpenEntryStreamAsync(CancellationToken.None); + }); + } + else if (archiveApi) + { + using var archive = ArchiveFactory.OpenArchive(input, options); + var entry = archive.Entries.First(); + Assert.Equal(45056, entry.Size); + Assert.Equal(0, entry.CompressedSize); + exception = Assert.Throws(() => + { + using var stream = entry.OpenEntryStream(); + }); + } + else if (useAsync) + { + using var forwardOnly = new NonSeekableStream(input); + await using var reader = await ReaderFactory.OpenAsyncReader( + forwardOnly, + options, + CancellationToken.None + ); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + exception = await Assert.ThrowsAsync(async () => + await reader.WriteEntryToAsync(Stream.Null, CancellationToken.None) + ); + } + else + { + using var forwardOnly = new NonSeekableStream(input); + using var reader = ReaderFactory.OpenReader(forwardOnly, options); + Assert.True(reader.MoveToNextEntry()); + exception = Assert.Throws(() => + reader.WriteEntryTo(Stream.Null) + ); + } + Assert.Contains("RAR5 dictionary size", exception.Message); + Assert.Contains(DictionarySizeLimits.Rar5.ToString(), exception.Message); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Rar5_Custom_Limit_Accounts_For_Minimum_Window_Allocation(bool useAsync) + { + using var limits = new DictionarySizeLimitScope(); + var bytes = DecodeHex(Rar256MiB); + // Change the compression-info vint from 0x2c80 to 0x0080: a 128 KiB + // declared dictionary. The unpacker requires at least a 256 KiB window. + bytes[46] = 1; + var crc = ~Crc32.Update(Crc32.DefaultSeed, bytes.AsSpan(29)); + BinaryPrimitives.WriteUInt32LittleEndian(bytes.AsSpan(25), crc); + foreach (var maximum in new[] { 128L << 10, 256L << 10 }) + { + using var input = new MemoryStream(bytes); + DictionarySizeLimits.Rar5 = maximum; + var options = ReaderOptions.ForExternalStream; + if (useAsync) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + input, + options, + CancellationToken.None + ); + var entry = await archive.EntriesAsync.FirstAsync(); + if (maximum < 256L << 10) + { + await Assert.ThrowsAsync(async () => + { + using var stream = await entry.OpenEntryStreamAsync(CancellationToken.None); + }); + } + else + { + using var stream = await entry.OpenEntryStreamAsync(CancellationToken.None); + } + } + else + { + using var archive = ArchiveFactory.OpenArchive(input, options); + var entry = archive.Entries.First(); + if (maximum < 256L << 10) + { + Assert.Throws(() => + { + using var stream = entry.OpenEntryStream(); + }); + } + else + { + using var stream = entry.OpenEntryStream(); + } + } + } + } + + private static byte[] DecodeHex(string hex) => + Enumerable + .Range(0, hex.Length / 2) + .Select(i => Convert.ToByte(hex.Substring(i * 2, 2), 16)) + .ToArray(); +} diff --git a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs new file mode 100644 index 00000000..6b5e853e --- /dev/null +++ b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs @@ -0,0 +1,389 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Compressors; +using SharpCompress.Compressors.LZMA; +using SharpCompress.Compressors.PPMd; +using SharpCompress.Compressors.PPMd.H; +using SharpCompress.Compressors.ZStandard; +using SharpCompress.Compressors.ZStandard.Unsafe; +using SharpCompress.Readers; +using Xunit; + +namespace SharpCompress.Test.Streams; + +// Static settings must not change while other collections are extracting archives. +[CollectionDefinition(Name, DisableParallelization = true)] +public class DictionarySizeLimitCollection +{ + public const string Name = "Dictionary size limits"; +} + +internal sealed class DictionarySizeLimitScope : IDisposable +{ + private readonly long rar4 = DictionarySizeLimits.Rar4; + private readonly long rar5 = DictionarySizeLimits.Rar5; + private readonly long lzma = DictionarySizeLimits.Lzma; + private readonly long lzma2 = DictionarySizeLimits.Lzma2; + private readonly long ppmd = DictionarySizeLimits.Ppmd; + private readonly long zStandard = DictionarySizeLimits.ZStandard; + + public void Dispose() + { + DictionarySizeLimits.Rar4 = rar4; + DictionarySizeLimits.Rar5 = rar5; + DictionarySizeLimits.Lzma = lzma; + DictionarySizeLimits.Lzma2 = lzma2; + DictionarySizeLimits.Ppmd = ppmd; + DictionarySizeLimits.ZStandard = zStandard; + } +} + +[Collection(DictionarySizeLimitCollection.Name)] +public class DictionarySizeLimitTests : TestBase +{ + [Fact] + public void Static_Limits_Have_Format_Defaults_And_Reject_Nonpositive_Values() + { + using var limits = new DictionarySizeLimitScope(); + Assert.Equal(4L << 20, DictionarySizeLimits.Rar4); + Assert.Equal(64L << 20, DictionarySizeLimits.Rar5); + Assert.Equal(64L << 20, DictionarySizeLimits.Lzma); + Assert.Equal(64L << 20, DictionarySizeLimits.Lzma2); + Assert.Equal(64L << 20, DictionarySizeLimits.Ppmd); + Assert.Equal(128L << 20, DictionarySizeLimits.ZStandard); + DictionarySizeLimits.Rar5 = 256L << 20; + Assert.Equal(256L << 20, DictionarySizeLimits.Rar5); + Assert.Throws(() => DictionarySizeLimits.Rar4 = 0); + Assert.Throws(() => DictionarySizeLimits.Rar5 = -1); + Assert.Throws(() => DictionarySizeLimits.Lzma = 0); + Assert.Throws(() => DictionarySizeLimits.Lzma2 = -1); + Assert.Throws(() => DictionarySizeLimits.Ppmd = 0); + Assert.Throws(() => DictionarySizeLimits.ZStandard = -1); + Assert.Equal(256L << 20, DictionarySizeLimits.Rar5); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Lzma_And_Lzma2_Reject_Excessive_Declared_Dictionaries(bool useAsync) + { + foreach (var properties in new[] { new byte[] { 0x5d, 0, 0, 0, 0x10 }, new byte[] { 30 } }) + { + using var input = new MemoryStream(); + if (useAsync) + { + await Assert.ThrowsAsync(async () => + await LzmaStream.CreateAsync(properties, input, -1, -1) + ); + } + else + { + Assert.Throws(() => LzmaStream.Create(properties, input)); + } + Assert.True(input.CanRead); + Assert.Equal(0, input.Position); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Direct_Lzma_Decoder_Enforces_Default_Limit(bool useAsync) + { + using var decoder = new Decoder(); + decoder.SetDecoderProperties(new byte[] { 0x5d, 0, 0, 0, 0x10 }); + if (useAsync) + { + await Assert.ThrowsAsync(async () => + await decoder.CodeAsync( + Stream.Null, + Stream.Null, + 0, + 1, + null!, + CancellationToken.None + ) + ); + } + else + { + Assert.Throws(() => + decoder.Code(Stream.Null, Stream.Null, 0, 1, null!) + ); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Lzma2_Static_Limit_Is_Inclusive(bool useAsync) + { + using var limits = new DictionarySizeLimitScope(); + using var input = new MemoryStream(new byte[] { 1, 0, 0, 65, 0 }); + DictionarySizeLimits.Lzma2 = 4095; + if (useAsync) + { + await Assert.ThrowsAsync(async () => + await LzmaStream.CreateAsync(new byte[] { 0 }, input, -1, -1) + ); + DictionarySizeLimits.Lzma2 = 4096; + using var decoder = await LzmaStream.CreateAsync(new byte[] { 0 }, input, -1, -1); + var buffer = new byte[1]; + Assert.Equal(1, await decoder.ReadAsync(buffer, 0, 1, CancellationToken.None)); + Assert.Equal(65, buffer[0]); + } + else + { + Assert.Throws(() => LzmaStream.Create(new byte[] { 0 }, input)); + DictionarySizeLimits.Lzma2 = 4096; + using var decoder = LzmaStream.Create(new byte[] { 0 }, input); + Assert.Equal(65, decoder.ReadByte()); + } + } + + [Fact] + public void Ppmd_Properties_Reject_Large_Models_Before_Eager_Allocation() + { + Assert.Throws(() => new PpmdProperties(new byte[] { 0xf5, 0x0f })); + Assert.Throws(() => new PpmdProperties(256 << 20, 6)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZip_Ppmd_Rejects_Large_Models_Before_Allocation(bool useAsync) + { + var properties = new PpmdProperties(new byte[] { 6, 0, 0, 0, 0x10 }); + using var input = new MemoryStream(); + if (useAsync) + { + await Assert.ThrowsAsync(async () => + await PpmdStream.CreateAsync(properties, input, false, CancellationToken.None) + ); + } + else + { + Assert.Throws(() => + PpmdStream.Create(properties, input, false) + ); + } + Assert.True(input.CanRead); + } + + [Fact] + public void Rar_Ppmd_Suballocator_Enforces_Limit_Before_Renting() + { + using var limits = new DictionarySizeLimitScope(); + DictionarySizeLimits.Ppmd = 1 << 20; + using var allocator = new SubAllocator(); + Assert.Throws(() => allocator.StartSubAllocator(2 << 20)); + Assert.Equal(0, allocator.GetAllocatedMemory()); + Assert.True(allocator.StartSubAllocator(1 << 20)); + Assert.Equal(1 << 20, allocator.GetAllocatedMemory()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task LZip_Uses_Static_Lzma_Limit(bool useAsync) + { + var bytes = new byte[26]; + new byte[] { (byte)'L', (byte)'Z', (byte)'I', (byte)'P', 1, 28 }.CopyTo(bytes, 0); + using var input = new MemoryStream(bytes); + if (useAsync) + { + await Assert.ThrowsAsync(async () => + await LZipStream.CreateAsync( + input, + CompressionMode.Decompress, + cancellationToken: CancellationToken.None + ) + ); + } + else + { + Assert.Throws(() => + LZipStream.Create(input, CompressionMode.Decompress) + ); + } + Assert.True(input.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Zstandard_Enforces_Default_And_Exact_Static_Window_Limits(bool useAsync) + { + using var limits = new DictionarySizeLimitScope(); + var defaultMaximum = DictionarySizeLimits.ZStandard; + foreach (var maximum in new long?[] { null, 16383, 16384 }) + { + DictionarySizeLimits.ZStandard = maximum ?? defaultMaximum; + // A 1 GiB or 16 KiB window with a final empty raw block. + using var input = new MemoryStream( + new byte[] + { + 0x28, + 0xb5, + 0x2f, + 0xfd, + 0, + maximum is null ? (byte)0xa0 : (byte)0x20, + 1, + 0, + 0, + } + ); + using var decoder = new DecompressionStream(input); + var buffer = new byte[1]; + if (maximum == 16384) + { + var read = useAsync + ? await decoder.ReadAsync(buffer, 0, 1, CancellationToken.None) + : decoder.Read(buffer, 0, 1); + Assert.Equal(0, read); + } + else + { + var exception = useAsync + ? await Assert.ThrowsAsync(async () => + await decoder.ReadAsync(buffer, 0, 1, CancellationToken.None) + ) + : Assert.Throws(() => decoder.Read(buffer, 0, 1)); + Assert.Equal( + ZSTD_ErrorCode.ZSTD_error_frameParameter_windowTooLarge, + exception.Code + ); + } + } + } + + public static IEnumerable ArchiveCases() + { + foreach ( + var file in new[] + { + "Rar.rar", + "Rar4.rar", + "Rar5.rar", + "Rar5.solid.rar", + "Zip.lzma.zip", + "Zip.ppmd.zip", + "WinZip27_XZ.zipx", + "WinZip27_ZSTD.zipx", + "7Zip.LZMA.7z", + "7Zip.LZMA2.7z", + "7Zip.PPMd.7z", + "7Zip.ZSTD.7z", + "Tar.tar.xz", + "Tar.tar.lz", + "Tar.tar.zst", + } + ) + { + foreach (var archiveApi in new[] { false, true }) + { + foreach (var useAsync in new[] { false, true }) + { + if ( + (!archiveApi && file.EndsWith(".7z", StringComparison.Ordinal)) + || (archiveApi && file.StartsWith("Tar.", StringComparison.Ordinal)) + ) + { + continue; + } + yield return new object[] { file, archiveApi, useAsync }; + } + } + } + } + + [Theory] + [MemberData(nameof(ArchiveCases))] + public async Task Static_Limits_Apply_To_Archive_And_Reader_Paths( + string file, + bool archiveApi, + bool useAsync + ) + { + using (var limits = new DictionarySizeLimitScope()) + { + DictionarySizeLimits.Rar4 = + DictionarySizeLimits.Rar5 = + DictionarySizeLimits.Lzma = + DictionarySizeLimits.Lzma2 = + DictionarySizeLimits.Ppmd = + DictionarySizeLimits.ZStandard = + 1024; + await Assert.ThrowsAnyAsync(() => + Extract(file, archiveApi, useAsync) + ); + } + await Extract(file, archiveApi, useAsync); + } + + private static async Task Extract(string file, bool archiveApi, bool useAsync) + { + using var input = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, file)); + var options = ReaderOptions.ForExternalStream; + if (archiveApi && useAsync) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + input, + options, + CancellationToken.None + ); + await foreach (var entry in archive.EntriesAsync) + { + if (!entry.IsDirectory) + { + using var stream = await entry.OpenEntryStreamAsync(CancellationToken.None); + await stream.CopyToAsync(Stream.Null, 81920, CancellationToken.None); + } + } + } + else if (archiveApi) + { + using var archive = ArchiveFactory.OpenArchive(input, options); + foreach (var entry in archive.Entries.Where(e => !e.IsDirectory)) + { + using var stream = entry.OpenEntryStream(); + stream.CopyTo(Stream.Null); + } + } + else if (useAsync) + { + using var forwardOnly = new NonSeekableStream(input); + await using var reader = await ReaderFactory.OpenAsyncReader( + forwardOnly, + options, + CancellationToken.None + ); + while (await reader.MoveToNextEntryAsync(CancellationToken.None)) + { + if (!reader.Entry.IsDirectory) + { + await reader.WriteEntryToAsync(Stream.Null, CancellationToken.None); + } + } + } + else + { + using var forwardOnly = new NonSeekableStream(input); + using var reader = ReaderFactory.OpenReader(forwardOnly, options); + while (reader.MoveToNextEntry()) + { + if (!reader.Entry.IsDirectory) + { + reader.WriteEntryTo(Stream.Null); + } + } + } + } +} diff --git a/tests/SharpCompress.Test/Streams/LzmaDictionaryAllocationTests.cs b/tests/SharpCompress.Test/Streams/LzmaDictionaryAllocationTests.cs index fefb6432..00711389 100644 --- a/tests/SharpCompress.Test/Streams/LzmaDictionaryAllocationTests.cs +++ b/tests/SharpCompress.Test/Streams/LzmaDictionaryAllocationTests.cs @@ -10,6 +10,7 @@ using Xunit; namespace SharpCompress.Test.Streams; +[Collection(DictionarySizeLimitCollection.Name)] public class LzmaDictionaryAllocationTests { [Theory] @@ -22,6 +23,8 @@ public class LzmaDictionaryAllocationTests bool direct ) { + using var limits = new DictionarySizeLimitScope(); + DictionarySizeLimits.Lzma = int.MaxValue; var expected = new byte[20000]; new Random(123).NextBytes(expected); using var packed = new MemoryStream(); @@ -210,6 +213,8 @@ public class LzmaDictionaryAllocationTests [InlineData(true)] public async Task Lzma2_Tiny_Stream_With_Huge_Dictionary_Decodes(bool useAsync) { + using var limits = new DictionarySizeLimitScope(); + DictionarySizeLimits.Lzma2 = int.MaxValue; using var input = new MemoryStream(new byte[] { 1, 0, 0, 65, 0 }); using var output = new MemoryStream(); if (useAsync) From 09b95f18417a597ccfea6a0ac20bc12a48643b20 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:27:51 +0000 Subject: [PATCH 13/26] Fix PPMd properties state after rejected allocator changes --- .../Compressors/PPMd/PpmdProperties.cs | 5 +++-- .../Streams/DictionarySizeLimitTests.cs | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/Compressors/PPMd/PpmdProperties.cs b/src/SharpCompress/Compressors/PPMd/PpmdProperties.cs index e4d52c17..708bdd36 100644 --- a/src/SharpCompress/Compressors/PPMd/PpmdProperties.cs +++ b/src/SharpCompress/Compressors/PPMd/PpmdProperties.cs @@ -55,13 +55,14 @@ public class PpmdProperties get => _allocatorSize; set { - _allocatorSize = value; if (Version == PpmdVersion.I1) { _allocator ??= new Allocator(); - _allocator.Start(_allocatorSize); + _allocator.Start(value); } + // Keep the serialized properties in sync with the allocator if Start rejects the size. + _allocatorSize = value; } } diff --git a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs index 6b5e853e..2619761b 100644 --- a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs +++ b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs @@ -154,6 +154,23 @@ public class DictionarySizeLimitTests : TestBase Assert.Throws(() => new PpmdProperties(256 << 20, 6)); } + [Fact] + public void Ppmd_Properties_Preserve_Size_After_Rejected_Change() + { + using var limits = new DictionarySizeLimitScope(); + DictionarySizeLimits.Ppmd = 2 << 20; + var properties = new PpmdProperties(1 << 20, 6); + var originalHeader = properties.Properties; + + Assert.Throws(() => properties.AllocatorSize = 3 << 20); + Assert.Equal(1 << 20, properties.AllocatorSize); + Assert.Equal(originalHeader, properties.Properties); + + properties.AllocatorSize = 2 << 20; + Assert.Equal(2 << 20, properties.AllocatorSize); + Assert.Equal(2 << 20, new PpmdProperties(properties.Properties).AllocatorSize); + } + [Theory] [InlineData(false)] [InlineData(true)] From f2e4df509bd6c2a9cb939198699828f2da4f4257 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:28:53 +0000 Subject: [PATCH 14/26] Keep LZMA decoders retryable after dictionary limit rejection --- .../Compressors/LZMA/LzmaDecoder.cs | 6 +- .../Streams/DictionarySizeLimitTests.cs | 87 +++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs b/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs index 2cafa087..62e03f94 100644 --- a/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs +++ b/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs @@ -267,10 +267,12 @@ public partial class Decoder : ICoder, ISetDecoderProperties, IDisposable { throw new InvalidParamException(); } - _outWindow = new OutWindow(); var blockSize = Math.Max(_dictionarySize, (1 << 12)); DictionarySizeLimits.Validate(blockSize, DictionarySizeLimits.Lzma, "LZMA"); - _outWindow.Create(blockSize); + // Publish only a usable window so a rejected dictionary can be retried. + var window = new OutWindow(); + window.Create(blockSize); + _outWindow = window; } private void SetLiteralProperties(int lp, int lc) diff --git a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs index 2619761b..f9dbcd0b 100644 --- a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs +++ b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs @@ -119,6 +119,93 @@ public class DictionarySizeLimitTests : TestBase } } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task Direct_Lzma_Decoder_Can_Retry_After_Limit_Rejection(bool useAsync, bool train) + { + using var limits = new DictionarySizeLimitScope(); + var expected = new byte[128]; + using var packed = new MemoryStream(); + byte[] properties; + using ( + var encoder = LzmaStream.Create(new LzmaEncoderProperties(false, 4096), false, packed) + ) + { + properties = (byte[])encoder.Properties.Clone(); + encoder.Write(expected, 0, expected.Length); + } + packed.Position = 0; + using var output = new MemoryStream(); + using var decoder = new Decoder(); + decoder.SetDecoderProperties(properties); + DictionarySizeLimits.Lzma = 4095; + + if (useAsync) + { + await Assert.ThrowsAsync(async () => + { + if (train) + { + await decoder.TrainAsync(Stream.Null); + } + else + { + await decoder.CodeAsync( + packed, + output, + packed.Length, + expected.Length, + null!, + CancellationToken.None + ); + } + }); + } + else + { + Assert.Throws(() => + { + if (train) + { + decoder.Train(Stream.Null); + } + else + { + decoder.Code(packed, output, packed.Length, expected.Length, null!); + } + }); + } + Assert.Equal(0, packed.Position); + DictionarySizeLimits.Lzma = 4096; + if (useAsync) + { + if (train) + { + await decoder.TrainAsync(Stream.Null); + } + await decoder.CodeAsync( + packed, + output, + packed.Length, + expected.Length, + null!, + CancellationToken.None + ); + } + else + { + if (train) + { + decoder.Train(Stream.Null); + } + decoder.Code(packed, output, packed.Length, expected.Length, null!); + } + Assert.Equal(expected, output.ToArray()); + } + [Theory] [InlineData(false)] [InlineData(true)] From 85bf5141e6d199c40c93e3e7a1e3269bb3b9e49d Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:31:43 +0000 Subject: [PATCH 15/26] Reject unsupported Zstandard window limits at assignment --- docs/API.md | 4 +++- .../Common/DictionarySizeLimits.cs | 19 ++++++++++++++-- .../Streams/DictionarySizeLimitTests.cs | 22 +++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/docs/API.md b/docs/API.md index 089c58c5..bc297abf 100644 --- a/docs/API.md +++ b/docs/API.md @@ -393,7 +393,9 @@ No reader options or decoder overloads are needed. Settings require positive val invalid assignments throw `ArgumentOutOfRangeException` without changing the setting. Oversized dictionaries throw `InvalidFormatException`; Zstandard reports `ZstdException` with `ZSTD_error_frameParameter_windowTooLarge`. Zstandard's minimum -configurable window limit is 1 KiB. Format and engine restrictions still apply. +configurable window limit is 1 KiB, and its maximum is 1 GiB in a 32-bit process +or 2 GiB in a 64-bit process. Values outside that inclusive range are rejected +at assignment. Format and engine restrictions still apply. These are dictionary/window or model limits, not total-memory, output-size, or concurrency limits. Pool rounding and allocator overhead can increase actual memory. diff --git a/src/SharpCompress/Common/DictionarySizeLimits.cs b/src/SharpCompress/Common/DictionarySizeLimits.cs index 1ea21d62..5b59fac7 100644 --- a/src/SharpCompress/Common/DictionarySizeLimits.cs +++ b/src/SharpCompress/Common/DictionarySizeLimits.cs @@ -52,11 +52,26 @@ public static class DictionarySizeLimits set => SetMaximum(ref ppmd, value, nameof(Ppmd)); } - /// Zstandard streaming window limit. Defaults to 128 MiB. + /// + /// Zstandard streaming window limit. Defaults to 128 MiB. + /// Must be between 1 KiB and 1 GiB on 32-bit processes or 2 GiB on 64-bit processes. + /// public static long ZStandard { get => Interlocked.Read(ref zStandard); - set => SetMaximum(ref zStandard, value, nameof(ZStandard)); + set + { + // Match the engine bounds before replacing a working process-wide configuration. + var maximum = 1L << (IntPtr.Size == 4 ? 30 : 31); + if (value < 1024 || value > maximum) + { + throw new ArgumentOutOfRangeException( + nameof(ZStandard), + $"Zstandard window limits must be between 1024 and {maximum} bytes." + ); + } + SetMaximum(ref zStandard, value, nameof(ZStandard)); + } } private static void SetMaximum(ref long field, long value, string name) diff --git a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs index f9dbcd0b..f756f1d0 100644 --- a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs +++ b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs @@ -368,6 +368,28 @@ public class DictionarySizeLimitTests : TestBase } } + [Fact] + public void Zstandard_Rejects_Unsupported_Limits_Without_Changing_Configuration() + { + using var limits = new DictionarySizeLimitScope(); + var original = DictionarySizeLimits.ZStandard; + var maximum = 1L << (IntPtr.Size == 4 ? 30 : 31); + foreach (var invalid in new[] { 512L, 1023, maximum + 1, long.MaxValue }) + { + Assert.Throws(() => + DictionarySizeLimits.ZStandard = invalid + ); + Assert.Equal(original, DictionarySizeLimits.ZStandard); + using var decoder = new Decompressor(); + } + foreach (var valid in new[] { 1024L, maximum }) + { + DictionarySizeLimits.ZStandard = valid; + Assert.Equal(valid, DictionarySizeLimits.ZStandard); + using var decoder = new Decompressor(); + } + } + public static IEnumerable ArchiveCases() { foreach ( From b10627339c47f6c2c787f0a7d10904b861a0ad3c Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:32:21 +0000 Subject: [PATCH 16/26] Normalize TAR duplicate keys consistently with the writer --- .../Archives/AbstractWritableArchive.cs | 4 +- .../Tar/TarArchiveDuplicateTests.cs | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.cs b/src/SharpCompress/Archives/AbstractWritableArchive.cs index f2a7ac87..bcfc0594 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.cs @@ -156,9 +156,9 @@ public abstract partial class AbstractWritableArchive private string NormalizeKey(string key) { key = key.Replace('\\', '/'); - if (Type == ArchiveType.Zip) + if (Type is ArchiveType.Zip or ArchiveType.Tar) { - // ZipWriter removes drive prefixes and surrounding slashes when writing names. + // ZIP and TAR writers remove drive prefixes and surrounding slashes from names. #if LEGACY_DOTNET var colon = key.IndexOf(':'); #else diff --git a/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs b/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs index 6d4aa083..61b2d949 100644 --- a/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs +++ b/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs @@ -22,6 +22,57 @@ public class TarArchiveDuplicateTests archive.AddEntry("dir/file", stream, false); } + [Theory] + [InlineData("file.txt", "C:/file.txt", false)] + [InlineData("file.txt", "//file.txt/", false)] + [InlineData("dir/file.txt", "DIR\\FILE.TXT", false)] + [InlineData("file.txt", "C:/file.txt", true)] + [InlineData("file.txt", "//file.txt/", true)] + [InlineData("dir/file.txt", "DIR\\FILE.TXT", true)] + public async Task Tar_AddEntry_Rejects_Writer_Normalized_Duplicates( + string first, + string duplicate, + bool useAsync + ) + { + using var stream = new MemoryStream(); + if (useAsync) + { + await using var archive = await TarArchive.CreateAsyncArchive(); + await archive.AddEntryAsync(first, stream, false); + await Assert.ThrowsAsync(async () => + await archive.AddEntryAsync(duplicate, stream, false) + ); + } + else + { + using var archive = TarArchive.CreateArchive(); + archive.AddEntry(first, stream, false); + Assert.Throws(() => archive.AddEntry(duplicate, stream, false)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Tar_AddDirectory_Rejects_Trailing_Slash_Duplicates(bool useAsync) + { + if (useAsync) + { + await using var archive = await TarArchive.CreateAsyncArchive(); + await archive.AddDirectoryEntryAsync("folder"); + await Assert.ThrowsAsync(async () => + await archive.AddDirectoryEntryAsync("folder/") + ); + } + else + { + using var archive = TarArchive.CreateArchive(); + archive.AddDirectoryEntry("folder"); + Assert.Throws(() => archive.AddDirectoryEntry("folder/")); + } + } + [Fact] public async Task Tar_Async_ChecksPendingEntriesWhileRebuildingIsPaused() { From 7f7b8c7eda23bf4985f446f815672cc91ccc17ad Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:33:35 +0000 Subject: [PATCH 17/26] Validate effective TAR PAX sizes before streaming --- .../Common/Tar/TarHeaderFactory.Async.cs | 4 +- .../Common/Tar/TarHeaderFactory.cs | 4 +- .../Tar/TarSizeValidationTests.cs | 87 +++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs index ed5a62de..d1755ad4 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs @@ -32,6 +32,8 @@ internal static partial class TarHeaderFactory { yield break; } + // PAX metadata can replace the raw size; validate before either mode uses it. + var paddedSize = PadTo512(header.Size); switch (mode) { case StreamingMode.Seekable: @@ -39,7 +41,7 @@ internal static partial class TarHeaderFactory header.DataStartPosition = stream.Position; // Reject padded sizes or end offsets that would wrap into a backward seek. - stream.Position = checked(stream.Position + PadTo512(header.Size)); + stream.Position = checked(stream.Position + paddedSize); } break; case StreamingMode.Streaming: diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs index e72b23c1..e3a19d22 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs @@ -26,6 +26,8 @@ internal static partial class TarHeaderFactory { yield break; } + // PAX metadata can replace the raw size; validate before either mode uses it. + var paddedSize = PadTo512(header.Size); switch (mode) { case StreamingMode.Seekable: @@ -33,7 +35,7 @@ internal static partial class TarHeaderFactory header.DataStartPosition = stream.Position; // Reject padded sizes or end offsets that would wrap into a backward seek. - stream.Position = checked(stream.Position + PadTo512(header.Size)); + stream.Position = checked(stream.Position + paddedSize); } break; case StreamingMode.Streaming: diff --git a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs index a8e8c6f6..b25bdc51 100644 --- a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs +++ b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs @@ -1,5 +1,6 @@ using System; using System.Buffers.Binary; +using System.Globalization; using System.IO; using System.Text; using System.Threading; @@ -294,6 +295,92 @@ public class TarSizeValidationTests Assert.True(stream.CanRead); } + [Theory] + [InlineData(long.MaxValue, false)] + [InlineData(long.MaxValue, true)] + [InlineData(TarHeader.MAX_ENTRY_SIZE + 1, false)] + [InlineData(TarHeader.MAX_ENTRY_SIZE + 1, true)] + [InlineData(513L, false)] + [InlineData(513L, true)] + public async Task Tar_Reader_Validates_Effective_Pax_Size(long size, bool useAsync) + { + using var data = CreatePaxArchive(size); + using var stream = new ForwardOnlyStream(data); + var invalid = size > TarHeader.MAX_ENTRY_SIZE; + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var reader = await ReaderFactory.OpenAsyncReader( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + if (invalid) + { + await Assert.ThrowsAsync(async () => + await reader.MoveToNextEntryAsync(CancellationToken.None) + ); + } + else + { + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + Assert.Equal(size, reader.Entry.Size); + await reader.WriteEntryToAsync(Stream.Null, CancellationToken.None); + Assert.False(await reader.MoveToNextEntryAsync(CancellationToken.None)); + } + } + else + { + using var reader = ReaderFactory.OpenReader(stream, ReaderOptions.ForExternalStream); + Assert.True(reader.MoveToNextEntry()); + if (invalid) + { + Assert.Throws(() => reader.MoveToNextEntry()); + } + else + { + Assert.True(reader.MoveToNextEntry()); + Assert.Equal(size, reader.Entry.Size); + reader.WriteEntryTo(Stream.Null); + Assert.False(reader.MoveToNextEntry()); + } + } + Assert.True(stream.CanRead); + } + + private static MemoryStream CreatePaxArchive(long size) + { + var record = $" size={size.ToString(CultureInfo.InvariantCulture)}\n"; + var length = record.Length + 1; + while (length != record.Length + length.ToString(CultureInfo.InvariantCulture).Length) + { + length = record.Length + length.ToString(CultureInfo.InvariantCulture).Length; + } + var payload = Encoding.ASCII.GetBytes( + length.ToString(CultureInfo.InvariantCulture) + record + ); + var paxHeader = CreateHeader(payload.Length); + paxHeader[156] = (byte)'x'; + WriteChecksum(paxHeader); + var stream = new MemoryStream(); + var firstHeader = CreateHeader(0); + stream.Write(firstHeader, 0, firstHeader.Length); + stream.Write(paxHeader, 0, paxHeader.Length); + stream.Write(payload, 0, payload.Length); + stream.SetLength(stream.Length + ((512 - payload.Length % 512) % 512)); + stream.Position = stream.Length; + var header = CreateHeader(0); + stream.Write(header, 0, header.Length); + if (size >= 0 && size <= 1024) + { + stream.SetLength(stream.Length + ((size + 511) / 512 * 512)); + } + stream.SetLength(stream.Length + 1024); + stream.Position = 0; + return stream; + } + private static MemoryStream CreateArchive(long size) { var stream = new MemoryStream(); From 101c939cc60ad250dd93b6423ab01ebe14bdc51e Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:35:09 +0000 Subject: [PATCH 18/26] Exclude trusted LZMA2 writer trials from LZMA input limits --- .../Compressors/LZMA/Lzma2EncoderStream.cs | 4 +- .../Compressors/LZMA/LzmaDecoder.cs | 11 +++- .../Streams/DictionarySizeLimitTests.cs | 50 +++++++++++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/Compressors/LZMA/Lzma2EncoderStream.cs b/src/SharpCompress/Compressors/LZMA/Lzma2EncoderStream.cs index 67c6030a..f3cba24e 100644 --- a/src/SharpCompress/Compressors/LZMA/Lzma2EncoderStream.cs +++ b/src/SharpCompress/Compressors/LZMA/Lzma2EncoderStream.cs @@ -368,7 +368,9 @@ internal sealed class Lzma2EncoderStream : Stream props[3] = (byte)(_dictionarySize >> 16); props[4] = (byte)(_dictionarySize >> 24); - var decoder = _trialDecoder ??= new Decoder(); + // This decoder verifies our own output using trusted encoder properties. + // An independent LZMA input limit must not disable LZMA2 compression. + var decoder = _trialDecoder ??= new Decoder(enforceDictionarySizeLimit: false); decoder.SetDecoderProperties(props); decoder.Code(input, Stream.Null, input.Length, uncompressedSize, null); diff --git a/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs b/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs index 62e03f94..dce5673f 100644 --- a/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs +++ b/src/SharpCompress/Compressors/LZMA/LzmaDecoder.cs @@ -253,8 +253,14 @@ public partial class Decoder : ICoder, ISetDecoderProperties, IDisposable _rep2, _rep3; + private readonly bool _enforceDictionarySizeLimit; + public Decoder() + : this(enforceDictionarySizeLimit: true) { } + + internal Decoder(bool enforceDictionarySizeLimit) { + _enforceDictionarySizeLimit = enforceDictionarySizeLimit; _dictionarySize = -1; } @@ -268,7 +274,10 @@ public partial class Decoder : ICoder, ISetDecoderProperties, IDisposable throw new InvalidParamException(); } var blockSize = Math.Max(_dictionarySize, (1 << 12)); - DictionarySizeLimits.Validate(blockSize, DictionarySizeLimits.Lzma, "LZMA"); + if (_enforceDictionarySizeLimit) + { + DictionarySizeLimits.Validate(blockSize, DictionarySizeLimits.Lzma, "LZMA"); + } // Publish only a usable window so a rejected dictionary can be retried. var window = new OutWindow(); window.Create(blockSize); diff --git a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs index f756f1d0..4f363a76 100644 --- a/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs +++ b/tests/SharpCompress.Test/Streams/DictionarySizeLimitTests.cs @@ -234,6 +234,56 @@ public class DictionarySizeLimitTests : TestBase } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Lzma2_Encoding_Does_Not_Apply_The_Lzma_Input_Limit(bool useAsync) + { + using var limits = new DictionarySizeLimitScope(); + DictionarySizeLimits.Lzma = 4096; + DictionarySizeLimits.Lzma2 = 8192; + var expected = new byte[150000]; + using var packed = new MemoryStream(); + byte[] properties; + using (var encoder = new Lzma2EncoderStream(packed, 8192, 32)) + { + properties = (byte[])encoder.Properties.Clone(); + if (useAsync) + { + await encoder.WriteAsync(expected, 0, expected.Length, CancellationToken.None); + await encoder.DisposeAsync(); + } + else + { + encoder.Write(expected, 0, expected.Length); + } + } + Assert.True(packed.Length < expected.Length / 10, $"Encoded {packed.Length} bytes."); + packed.Position = 0; + using var output = new MemoryStream(); + if (useAsync) + { + await using var decoder = await LzmaStream.CreateAsync( + properties, + packed, + packed.Length, + expected.Length + ); + await decoder.CopyToAsync(output, 81920, CancellationToken.None); + } + else + { + using var decoder = LzmaStream.Create( + properties, + packed, + packed.Length, + expected.Length + ); + decoder.CopyTo(output); + } + Assert.Equal(expected, output.ToArray()); + } + [Fact] public void Ppmd_Properties_Reject_Large_Models_Before_Eager_Allocation() { From 2f040283fc53c7eec9e70118ed670302de4a7cb2 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:36:55 +0000 Subject: [PATCH 19/26] Preserve legacy 7z property size compatibility during preflight --- .../Common/SevenZip/ArchiveReader.Async.cs | 6 +- .../Common/SevenZip/ArchiveReader.cs | 18 +++- .../SevenZip/SevenZipHeaderAllocationTests.cs | 98 +++++++++++++++++++ 3 files changed, 119 insertions(+), 3 deletions(-) diff --git a/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs b/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs index d22faa16..ae1d141a 100644 --- a/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs +++ b/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs @@ -338,7 +338,11 @@ internal sealed partial class ArchiveReader } var numFiles = ReadNum(); - ValidateFileCount(numFiles, unpackSizes.Count); + ValidateFileCount( + numFiles, + unpackSizes.Count, + db._majorVersion > 0 || db._minorVersion > 2 + ); db._files = new List(numFiles); for (var i = 0; i < numFiles; i++) { diff --git a/src/SharpCompress/Common/SevenZip/ArchiveReader.cs b/src/SharpCompress/Common/SevenZip/ArchiveReader.cs index 8d5ae0e0..d2bb2e69 100644 --- a/src/SharpCompress/Common/SevenZip/ArchiveReader.cs +++ b/src/SharpCompress/Common/SevenZip/ArchiveReader.cs @@ -714,8 +714,18 @@ internal partial class ArchiveReader return (int)size; } - private void ValidateFileCount(int numFiles, int numStreams) + private void ValidateFileCount(int numFiles, int numStreams, bool checkRecordsSize) { + if (!checkRecordsSize) + { + // Before 0.3, known properties may have inaccurate sizes, so walking + // them by their declared lengths would disagree with the real parser. + // Bound the allocation by the minimum possible empty-stream bitmap; + // ReadBitVector separately bounds the actual bitmap at its read position. + var minimumEmptyStreams = Math.Max(0L, (long)numFiles - numStreams); + _currentReader.RequireBytes((minimumEmptyStreams + 7) / 8); + return; + } // Nonempty files correspond to already parsed substreams. Empty files // require one bit each in kEmptyStream, not one byte per file. Preflight // that property before allocating CFileItem objects or implicit vectors. @@ -914,7 +924,11 @@ internal partial class ArchiveReader } var numFiles = ReadNum(); - ValidateFileCount(numFiles, unpackSizes.Count); + ValidateFileCount( + numFiles, + unpackSizes.Count, + db._majorVersion > 0 || db._minorVersion > 2 + ); db._files = new List(numFiles); for (var i = 0; i < numFiles; i++) { diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs index 18db66b5..90f4dc24 100644 --- a/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs +++ b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs @@ -356,6 +356,104 @@ public class SevenZipHeaderAllocationTests Assert.True(stream.CanRead); } + [Theory] + [InlineData(2, 0, false)] + [InlineData(2, 0, true)] + [InlineData(2, 127, false)] + [InlineData(2, 127, true)] + [InlineData(3, 0, false)] + [InlineData(3, 0, true)] + public async Task SevenZipArchive_Property_Size_Validation_Is_Version_Aware( + byte minorVersion, + byte declaredSize, + bool useAsync + ) + { + // Both known properties have deliberately inaccurate sizes, including + // a name preceding the empty-stream bitmap. + using var stream = CreateArchive([ + 1, + 5, + 1, + 17, + declaredSize, + 0, + (byte)'x', + 0, + 0, + 0, + 14, + declaredSize, + 0x80, + 0, + 0, + ]); + stream.GetBuffer()[7] = minorVersion; + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + cancellationToken: CancellationToken.None + ); + if (minorVersion > 2) + { + await Assert.ThrowsAnyAsync(async () => + await archive.EntriesAsync.ToListAsync(CancellationToken.None) + ); + } + else + { + var entry = Assert.Single( + await archive.EntriesAsync.ToListAsync(CancellationToken.None) + ); + Assert.True(entry.IsDirectory); + Assert.Equal("x", entry.Key); + } + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream); + if (minorVersion > 2) + { + Assert.ThrowsAny(() => archive.Entries.ToList()); + } + else + { + var entry = Assert.Single(archive.Entries); + Assert.True(entry.IsDirectory); + Assert.Equal("x", entry.Key); + } + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SevenZipArchive_Legacy_File_Count_Is_Bounded_By_Available_Metadata( + bool useAsync + ) + { + using var stream = CreateArchive(Join([1, 5], Number(100_000), [14, 0, 0x80, 0])); + stream.GetBuffer()[7] = 2; + if (useAsync) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + stream, + cancellationToken: CancellationToken.None + ); + await Assert.ThrowsAsync(async () => + await archive.EntriesAsync.ToListAsync(CancellationToken.None) + ); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream); + Assert.Throws(() => archive.Entries.ToList()); + } + } + private static byte[] MetadataHeader( bool encoded, ulong unpackSize, From e19010b184e57c1ed6c3c861e6a0a63ed1f30645 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:38:46 +0000 Subject: [PATCH 20/26] Validate LZMA2 dictionary limits before parallel workers --- .../Compressors/LZMA/Lzma2ParallelDecoder.cs | 3 + .../Compressors/LZMA/LzmaStream.cs | 21 ++++-- .../Lzma2ParallelDictionaryLimitTests.cs | 74 +++++++++++++++++++ 3 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 tests/SharpCompress.Test/Streams/Lzma2ParallelDictionaryLimitTests.cs diff --git a/src/SharpCompress/Compressors/LZMA/Lzma2ParallelDecoder.cs b/src/SharpCompress/Compressors/LZMA/Lzma2ParallelDecoder.cs index cb63751b..c2852ff5 100644 --- a/src/SharpCompress/Compressors/LZMA/Lzma2ParallelDecoder.cs +++ b/src/SharpCompress/Compressors/LZMA/Lzma2ParallelDecoder.cs @@ -250,6 +250,9 @@ internal static class Lzma2ParallelDecoder { ThrowHelper.ThrowIfNull(bufferPool); + // Reject folder-wide properties here, not inside workers where Parallel + // would wrap the format exception in AggregateException. + LzmaStream.GetLzma2DictionarySize(lzma2Props[0]); Parallel.ForEach( blocks, new ParallelOptions { MaxDegreeOfParallelism = maxDegreeOfParallelism }, diff --git a/src/SharpCompress/Compressors/LZMA/LzmaStream.cs b/src/SharpCompress/Compressors/LZMA/LzmaStream.cs index 90e14c5f..e82f9816 100644 --- a/src/SharpCompress/Compressors/LZMA/LzmaStream.cs +++ b/src/SharpCompress/Compressors/LZMA/LzmaStream.cs @@ -78,14 +78,7 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable } else { - // Larger standard dictionaries exceed this decoder's positive Int32 window range. - if (properties[0] > 37) - { - throw new InvalidParamException(); - } - _dictionarySize = 2 | (properties[0] & 1); - _dictionarySize <<= (properties[0] >> 1) + 11; - DictionarySizeLimits.Validate(_dictionarySize, DictionarySizeLimits.Lzma2, "LZMA2"); + _dictionarySize = GetLzma2DictionarySize(properties[0]); Properties = new byte[1]; _availableBytes = 0; @@ -93,6 +86,18 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable } } + internal static int GetLzma2DictionarySize(byte property) + { + // Larger standard dictionaries exceed this decoder's positive Int32 window range. + if (property > 37) + { + throw new InvalidParamException(); + } + var size = (2 | (property & 1)) << ((property >> 1) + 11); + DictionarySizeLimits.Validate(size, DictionarySizeLimits.Lzma2, "LZMA2"); + return size; + } + public static LzmaStream Create( byte[] properties, Stream inputStream, diff --git a/tests/SharpCompress.Test/Streams/Lzma2ParallelDictionaryLimitTests.cs b/tests/SharpCompress.Test/Streams/Lzma2ParallelDictionaryLimitTests.cs new file mode 100644 index 00000000..f1d3d2c8 --- /dev/null +++ b/tests/SharpCompress.Test/Streams/Lzma2ParallelDictionaryLimitTests.cs @@ -0,0 +1,74 @@ +using System; +using System.IO; +using SharpCompress.Archives.SevenZip; +using SharpCompress.Common; +using SharpCompress.Compressors.LZMA; +using SharpCompress.Readers; +using SharpCompress.Writers.SevenZip; +using Xunit; + +namespace SharpCompress.Test.Streams; + +[Collection(DictionarySizeLimitCollection.Name)] +public class Lzma2ParallelDictionaryLimitTests : TestBase +{ + [Theory] + [InlineData(false)] + [InlineData(true)] + public void SevenZip_File_Extraction_Reports_Dictionary_Limits_Consistently(bool parallel) + { + using var limits = new DictionarySizeLimitScope(); + var path = Path.Combine(SCRATCH_FILES_PATH, "parallel-dictionary-limit.7z"); + using (var output = File.Create(path)) + using ( + var writer = new SevenZipWriter( + output, + new SevenZipWriterOptions(CompressionType.LZMA2) + { + CompressHeader = false, + LzmaProperties = new LzmaEncoderProperties(false, 8192, 32), + } + ) + ) + { + // Two full-reset chunks make the file-backed parallel path eligible. + using var source = new MemoryStream(new byte[(2 << 20) + (1 << 16)]); + writer.Write("payload.bin", source, DateTime.UtcNow); + } + DictionarySizeLimits.Lzma2 = 4096; + using var archive = SevenZipArchive.OpenArchive( + path, + ReaderOptions.ForFilePath.WithEnableParallelism(parallel) + ); + var entry = Assert.Single(archive.Entries); + Assert.Throws(() => + { + using var stream = entry.OpenEntryStream(); + stream.CopyTo(Stream.Null); + }); + } + +#if !LEGACY_DOTNET + [Fact] + public void Parallel_Dictionary_Validation_Runs_Before_Workers() + { + using var limits = new DictionarySizeLimitScope(); + DictionarySizeLimits.Lzma2 = 4096; + var path = Path.Combine(SCRATCH_FILES_PATH, "parallel-dictionary-workers.bin"); + using var input = File.Create(path); + using var output = File.Create(path + ".out"); + var blocks = new[] { new Lzma2Block(0, 1, 0, 1), new Lzma2Block(1, 1, 1, 1) }; + Assert.Throws(() => + Lzma2ParallelDecoder.DecodeBlocksParallel( + input.SafeFileHandle, + new byte[] { 2 }, + 0, + blocks, + output.SafeFileHandle, + 2 + ) + ); + Assert.Equal(0, output.Length); + } +#endif +} From da868e2f476c9cecd67f53f2c02d8e89a181fe78 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:44:27 +0000 Subject: [PATCH 21/26] Share writable archive entry identities across sync and async APIs --- .../Archives/AbstractWritableArchive.Async.cs | 48 ++++- .../Archives/AbstractWritableArchive.cs | 59 +++++- .../GZip/GZipArchiveAsyncTests.cs | 3 +- .../WritableArchiveIdentityTests.cs | 189 ++++++++++++++++++ 4 files changed, 292 insertions(+), 7 deletions(-) create mode 100644 tests/SharpCompress.Test/WritableArchiveIdentityTests.cs diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs index 4b29c4ca..4548a21c 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs @@ -37,9 +37,52 @@ public abstract partial class AbstractWritableArchive } } - // Async property moved from main file private IAsyncEnumerable OldEntriesAsync => - base.EntriesAsync.Where(x => !removedEntries.Contains(x)); + LoadOriginalEntriesAsync().Where(x => !removedEntries.Contains(x)); + + private async IAsyncEnumerable LoadOriginalEntriesAsync( + [EnumeratorCancellation] CancellationToken cancellationToken = default + ) + { + var index = 0; + while (true) + { + while (index < originalEntries.Count) + { + cancellationToken.ThrowIfCancellationRequested(); + yield return originalEntries[index++]; + } + if (originalEntriesLoaded) + { + yield break; + } + + var generation = ++originalEntriesGeneration; + await using var source = LoadEntriesAsync(VolumesAsync) + .GetAsyncEnumerator(cancellationToken); + for (var skipped = 0; skipped < index; skipped++) + { + if (!await source.MoveNextAsync().ConfigureAwait(false)) + { + throw new IncompleteArchiveException("Archive entries changed while loading."); + } + } + while ( + generation == originalEntriesGeneration + && await source.MoveNextAsync().ConfigureAwait(false) + ) + { + cancellationToken.ThrowIfCancellationRequested(); + originalEntries.Add(source.Current); + index++; + yield return source.Current; + } + if (generation == originalEntriesGeneration) + { + originalEntriesLoaded = true; + } + } + } private async ValueTask RebuildModifiedCollectionAsync() { @@ -155,6 +198,7 @@ public abstract partial class AbstractWritableArchive public override async ValueTask DisposeAsync() { await base.DisposeAsync().ConfigureAwait(false); + originalEntries.Cast().ForEach(x => x.Close()); newEntries.Cast().ForEach(x => x.Close()); removedEntries.Cast().ForEach(x => x.Close()); modifiedEntries.Cast().ForEach(x => x.Close()); diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.cs b/src/SharpCompress/Archives/AbstractWritableArchive.cs index bcfc0594..dea9de08 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.cs @@ -41,6 +41,10 @@ public abstract partial class AbstractWritableArchive private readonly List newEntries = new(); private readonly List removedEntries = new(); + private readonly List originalEntries = new(); + private readonly LazyReadOnlyCollection originalEntriesView; + private bool originalEntriesLoaded; + private int originalEntriesGeneration; private readonly List modifiedEntries = new(); private bool hasModifications; @@ -48,10 +52,17 @@ public abstract partial class AbstractWritableArchive private bool asyncRebuildPending; internal AbstractWritableArchive(ArchiveType type) - : base(type) { } + : base(type) + { + originalEntriesLoaded = true; + originalEntriesView = new LazyReadOnlyCollection(LoadOriginalEntries()); + } internal AbstractWritableArchive(ArchiveType type, SourceStream sourceStream) - : base(type, sourceStream) { } + : base(type, sourceStream) + { + originalEntriesView = new LazyReadOnlyCollection(LoadOriginalEntries()); + } public override ICollection Entries { @@ -65,7 +76,7 @@ public abstract partial class AbstractWritableArchive { return modifiedEntries; } - return base.Entries; + return originalEntriesView; } } @@ -84,7 +95,46 @@ public abstract partial class AbstractWritableArchive modifiedEntries.AddRange(OldEntries.Concat(newEntries)); } - private IEnumerable OldEntries => base.Entries.Where(x => !removedEntries.Contains(x)); + private IEnumerable LoadOriginalEntries() + { + var index = 0; + while (true) + { + while (index < originalEntries.Count) + { + yield return originalEntries[index++]; + } + if (originalEntriesLoaded) + { + yield break; + } + + // Both APIs share entry identities. Switching loaders must restart + // parsing: the other loader may have moved the archive's source stream. + var generation = ++originalEntriesGeneration; + using var source = LoadEntries(Volumes).GetEnumerator(); + for (var skipped = 0; skipped < index; skipped++) + { + if (!source.MoveNext()) + { + throw new IncompleteArchiveException("Archive entries changed while loading."); + } + } + while (generation == originalEntriesGeneration && source.MoveNext()) + { + originalEntries.Add(source.Current); + index++; + yield return source.Current; + } + if (generation == originalEntriesGeneration) + { + originalEntriesLoaded = true; + } + } + } + + private IEnumerable OldEntries => + originalEntriesView.Where(x => !removedEntries.Contains(x)); public void RemoveEntry(TEntry entry) { @@ -256,6 +306,7 @@ public abstract partial class AbstractWritableArchive public override void Dispose() { base.Dispose(); + originalEntries.Cast().ForEach(x => x.Close()); newEntries.Cast().ForEach(x => x.Close()); removedEntries.Cast().ForEach(x => x.Close()); modifiedEntries.Cast().ForEach(x => x.Close()); diff --git a/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs b/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs index ba5a7782..93651949 100644 --- a/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs +++ b/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs @@ -85,7 +85,8 @@ public class GZipArchiveAsyncTests : ArchiveTests #endif await using (var archive = await GZipArchive.OpenAsyncArchive(new AsyncOnlyStream(stream))) { - await Assert.ThrowsAsync(async () => + // Shared cached entries avoid synchronous I/O on the async-only source. + await Assert.ThrowsAsync(async () => await archive.AddEntryAsync("jpg\\test.jpg", File.OpenRead(jpg), closeStream: true) ); await archive.SaveToAsync( diff --git a/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs b/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs new file mode 100644 index 00000000..aad0fd86 --- /dev/null +++ b/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs @@ -0,0 +1,189 @@ +using System; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Archives.Tar; +using SharpCompress.Archives.Zip; +using SharpCompress.Common; +using SharpCompress.Readers; +using SharpCompress.Writers; +using SharpCompress.Writers.Tar; +using SharpCompress.Writers.Zip; +using Xunit; + +namespace SharpCompress.Test; + +public class WritableArchiveIdentityTests +{ + [Theory] + [InlineData(ArchiveType.Zip, false, false)] + [InlineData(ArchiveType.Zip, false, true)] + [InlineData(ArchiveType.Zip, true, false)] + [InlineData(ArchiveType.Zip, true, true)] + [InlineData(ArchiveType.Tar, false, false)] + [InlineData(ArchiveType.Tar, false, true)] + [InlineData(ArchiveType.Tar, true, false)] + [InlineData(ArchiveType.Tar, true, true)] + public async Task Removal_Survives_Switching_Apis_And_Rebuilding( + ArchiveType type, + bool asyncFirst, + bool paused + ) + { + using var input = CreateArchive(type, "remove", "keep"); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + var writable = (IWritableArchive)archive; + var asyncWritable = (IWritableAsyncArchive)archive; + // Leave the first loader partially consumed before switching APIs. + var entry = asyncFirst + ? await asyncWritable.EntriesAsync.FirstAsync(CancellationToken.None) + : archive.Entries.First(); + var otherEntry = asyncFirst + ? archive.Entries.First() + : await asyncWritable.EntriesAsync.FirstAsync(CancellationToken.None); + Assert.Same(entry, otherEntry); + + using var source = new MemoryStream(new byte[] { 42 }); + using (paused ? writable.PauseEntryRebuilding() : null) + { + if (asyncFirst) + { + writable.RemoveEntry(entry); + writable.AddEntry("new", source, false); + } + else + { + await asyncWritable.RemoveEntryAsync(entry); + await asyncWritable.AddEntryAsync( + "new", + source, + false, + cancellationToken: CancellationToken.None + ); + } + } + var syncEntries = archive.Entries.ToList(); + var asyncEntries = await asyncWritable.EntriesAsync.ToListAsync(CancellationToken.None); + Assert.Equal(new[] { "keep", "new" }, syncEntries.Select(e => e.Key)); + Assert.Equal(syncEntries, asyncEntries); + + foreach (var useAsync in new[] { false, true }) + { + using var output = new MemoryStream(); + await Save(archive, output, useAsync); + output.Position = 0; + using var saved = ArchiveFactory.OpenArchive(output, ReaderOptions.ForExternalStream); + Assert.Equal(new[] { "keep", "new" }, saved.Entries.Select(e => e.Key)); + } + Assert.True(input.CanRead); + Assert.True(source.CanRead); + } + + [Theory] + [InlineData(ArchiveType.Zip, false)] + [InlineData(ArchiveType.Zip, true)] + [InlineData(ArchiveType.Tar, false)] + [InlineData(ArchiveType.Tar, true)] + public async Task Shared_Identity_Does_Not_Remove_Other_Entries_With_The_Same_Name( + ArchiveType type, + bool asyncFirst + ) + { + using var input = CreateArchive(type, "same", "same"); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + var asyncArchive = (IWritableAsyncArchive)archive; + if (asyncFirst) + { + var entry = await asyncArchive.EntriesAsync.FirstAsync(CancellationToken.None); + ((IWritableArchive)archive).RemoveEntry(entry); + } + else + { + await asyncArchive.RemoveEntryAsync(archive.Entries.First()); + } + var remaining = Assert.Single(archive.Entries); + Assert.Same( + remaining, + Assert.Single(await asyncArchive.EntriesAsync.ToListAsync(CancellationToken.None)) + ); + using var payload = remaining.OpenEntryStream(); + Assert.Equal(2, payload.ReadByte()); + } + + [Theory] + [InlineData(ArchiveType.Zip)] + [InlineData(ArchiveType.Tar)] + public async Task Partially_Consumed_Loaders_Resume_From_The_Shared_Cache(ArchiveType type) + { + using var input = CreateArchive(type, "first", "second", "third"); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + using var syncEntries = archive.Entries.GetEnumerator(); + await using var asyncEntries = ((IAsyncArchive)archive).EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + Assert.True(syncEntries.MoveNext()); + Assert.True(await asyncEntries.MoveNextAsync()); + Assert.Same(syncEntries.Current, asyncEntries.Current); + Assert.True(await asyncEntries.MoveNextAsync()); + var second = asyncEntries.Current; + Assert.True(syncEntries.MoveNext()); + Assert.Same(second, syncEntries.Current); + Assert.True(syncEntries.MoveNext()); + Assert.True(await asyncEntries.MoveNextAsync()); + Assert.Same(syncEntries.Current, asyncEntries.Current); + Assert.False(syncEntries.MoveNext()); + Assert.False(await asyncEntries.MoveNextAsync()); + } + + private static MemoryStream CreateArchive(ArchiveType type, params string[] names) + { + var input = new MemoryStream(); + using ( + var writer = WriterFactory.OpenWriter( + input, + type, + new WriterOptions(CompressionType.None) { LeaveStreamOpen = true } + ) + ) + { + for (var i = 0; i < names.Length; i++) + { + using var source = new MemoryStream(new[] { (byte)(i + 1) }); + writer.Write(names[i], source, DateTime.UtcNow); + } + } + input.Position = 0; + return input; + } + + private static async Task Save(IArchive archive, Stream output, bool useAsync) + { + if (archive is ZipArchive zip) + { + var options = new ZipWriterOptions(CompressionType.None) { LeaveStreamOpen = true }; + if (useAsync) + { + await zip.SaveToAsync(output, options, CancellationToken.None); + } + else + { + zip.SaveTo(output, options); + } + } + else + { + var tar = (TarArchive)archive; + var options = new TarWriterOptions(CompressionType.None) { LeaveStreamOpen = true }; + if (useAsync) + { + await tar.SaveToAsync(output, options, CancellationToken.None); + } + else + { + tar.SaveTo(output, options); + } + } + } +} From d4db8abac887d62f56705c859f9a9b8d12683a52 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 09:59:57 +0000 Subject: [PATCH 22/26] Parse decoded 7z metadata directly from pooled storage --- .../Common/SevenZip/ArchiveReader.Async.cs | 81 +++++++------ .../Common/SevenZip/ArchiveReader.cs | 112 ++++++++++++------ .../Common/SevenZip/CStreamSwitch.cs | 16 ++- .../Common/SevenZip/DataReader.cs | 102 ++++++++++++---- src/SharpCompress/IO/PooledMemoryStream.cs | 22 ++++ .../SevenZip/SevenZipDataReaderTests.cs | 67 +++++++++++ .../SevenZip/SevenZipHeaderLifetimeTests.cs | 112 ++++++++++++++++++ 7 files changed, 416 insertions(+), 96 deletions(-) create mode 100644 tests/SharpCompress.Test/SevenZip/SevenZipDataReaderTests.cs diff --git a/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs b/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs index ae1d141a..7c7ffad7 100644 --- a/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs +++ b/src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs @@ -128,20 +128,22 @@ internal sealed partial class ArchiveReader streamSwitch.Set(this, header); var type = ReadId(); - if (type != BlockType.Header) + if (type is not BlockType.Header and not BlockType.EncodedHeader) { - if (type != BlockType.EncodedHeader) - { - throw new ArchiveOperationException(); - } - - var dataVector = await ReadAndDecodePackedStreamsAsync( - db._startPositionAfterHeader, - db.PasswordProvider, - cancellationToken - ) - .ConfigureAwait(false); + throw new ArchiveOperationException(); + } + using var dataVector = + type == BlockType.EncodedHeader + ? await ReadAndDecodePackedStreamsAsync( + db._startPositionAfterHeader, + db.PasswordProvider, + cancellationToken + ) + .ConfigureAwait(false) + : null; + if (dataVector is not null) + { // compressed header without content is odd but ok if (dataVector.Count == 0) { @@ -168,12 +170,14 @@ internal sealed partial class ArchiveReader return db; } - private async ValueTask> ReadAndDecodePackedStreamsAsync( + private async ValueTask ReadAndDecodePackedStreamsAsync( long baseOffset, IPasswordProvider pass, CancellationToken cancellationToken ) { + DecodedMetadataStreams? dataVector = null; + var buffer = decoderBufferPool.Rent(MetadataReadBufferSize); try { ReadStreamsInfo( @@ -189,7 +193,7 @@ internal sealed partial class ArchiveReader dataStartPos += baseOffset; - var dataVector = new List(folders.Count); + dataVector = new DecodedMetadataStreams(folders.Count); var packIndex = 0; foreach (var folder in folders) { @@ -220,17 +224,22 @@ internal sealed partial class ArchiveReader // Grow only as the decoder actually produces metadata, never // from the untrusted unpack-size declaration alone. - using var decoded = new PooledMemoryStream(); - var buffer = new byte[8192]; + var decoded = new PooledMemoryStream(0, Constants.BufferSize, decoderBufferPool); + dataVector.Add(decoded); + var crc = Crc.INIT_CRC; for (var remaining = unpackSize; remaining > 0; ) { - var count = Math.Min(buffer.Length, remaining); + var count = Math.Min(MetadataReadBufferSize, remaining); await outStream .ReadExactAsync(buffer, 0, count, cancellationToken) .ConfigureAwait(false); await decoded .WriteAsync(buffer, 0, count, cancellationToken) .ConfigureAwait(false); + if (folder.UnpackCrcDefined) + { + crc = Crc.Update(crc, buffer, 0, count); + } remaining -= count; } if ( @@ -240,15 +249,9 @@ internal sealed partial class ArchiveReader { throw new InvalidFormatException("Decoded stream is longer than expected."); } - var data = decoded.ToArray(); - dataVector.Add(data); - if (folder.UnpackCrcDefined) { - if ( - Crc.Finish(Crc.Update(Crc.INIT_CRC, data, 0, unpackSize)) - != folder._unpackCrc - ) + if (Crc.Finish(crc) != folder._unpackCrc) { var exception = new InvalidFormatException( "Decoded stream does not match expected CRC." @@ -261,7 +264,15 @@ internal sealed partial class ArchiveReader } return dataVector; } - finally { } + catch + { + dataVector?.Dispose(); + throw; + } + finally + { + decoderBufferPool.Return(buffer); + } } private async ValueTask ReadHeaderAsync( @@ -280,15 +291,17 @@ internal sealed partial class ArchiveReader type = ReadId(); } - List? dataVector = null; - if (type == BlockType.AdditionalStreamsInfo) + using var dataVector = + type == BlockType.AdditionalStreamsInfo + ? await ReadAndDecodePackedStreamsAsync( + db._startPositionAfterHeader, + getTextPassword, + cancellationToken + ) + .ConfigureAwait(false) + : null; + if (dataVector is not null) { - dataVector = await ReadAndDecodePackedStreamsAsync( - db._startPositionAfterHeader, - getTextPassword, - cancellationToken - ) - .ConfigureAwait(false); type = ReadId(); } @@ -369,7 +382,7 @@ internal sealed partial class ArchiveReader case BlockType.Name: using (var streamSwitch = new CStreamSwitch()) { - streamSwitch.Set(this, dataVector ?? []); + streamSwitch.Set(this, dataVector); for (var i = 0; i < db._files.Count; i++) { db._files[i].Name = _currentReader.ReadString(); diff --git a/src/SharpCompress/Common/SevenZip/ArchiveReader.cs b/src/SharpCompress/Common/SevenZip/ArchiveReader.cs index d2bb2e69..e3b61f6a 100644 --- a/src/SharpCompress/Common/SevenZip/ArchiveReader.cs +++ b/src/SharpCompress/Common/SevenZip/ArchiveReader.cs @@ -16,6 +16,7 @@ namespace SharpCompress.Common.SevenZip; internal partial class ArchiveReader { + private const int MetadataReadBufferSize = 8192; private readonly ArrayPool decoderBufferPool; internal ArchiveReader(ArrayPool? decoderBufferPool = null) => @@ -36,6 +37,12 @@ internal partial class ArchiveReader _currentReader = new DataReader(buffer, offset, length); } + internal void AddByteStream(PooledMemoryStream buffer) + { + _readerStack.Push(_currentReader); + _currentReader = new DataReader(buffer); + } + internal void DeleteByteStream() => _currentReader = _readerStack.Pop(); #region Private Methods - Data Reader @@ -134,12 +141,16 @@ internal partial class ArchiveReader return ReadBitVector(length); } - private void ReadNumberVector(List? dataVector, int numFiles, Action action) + private void ReadNumberVector( + List? dataVector, + int numFiles, + Action action + ) { var defined = ReadOptionalBitVector(numFiles); using var streamSwitch = new CStreamSwitch(); - streamSwitch.Set(this, dataVector ?? []); + streamSwitch.Set(this, dataVector); for (var i = 0; i < numFiles; i++) { @@ -168,7 +179,7 @@ internal partial class ArchiveReader } private void ReadDateTimeVector( - List? dataVector, + List? dataVector, int numFiles, Action action ) => @@ -179,14 +190,14 @@ internal partial class ArchiveReader ); private void ReadAttributeVector( - List? dataVector, + List? dataVector, int numFiles, Action action ) { var boolVector = ReadOptionalBitVector(numFiles); using var streamSwitch = new CStreamSwitch(); - streamSwitch.Set(this, dataVector ?? []); + streamSwitch.Set(this, dataVector); for (var i = 0; i < numFiles; i++) { if (boolVector[i]) @@ -431,7 +442,7 @@ internal partial class ArchiveReader finally { } } - private void ReadUnpackInfo(List? dataVector, out List folders) + private void ReadUnpackInfo(List? dataVector, out List folders) { try { @@ -440,7 +451,7 @@ internal partial class ArchiveReader using (var streamSwitch = new CStreamSwitch()) { - streamSwitch.Set(this, dataVector ?? []); + streamSwitch.Set(this, dataVector); // Folder definitions may live in an external metadata stream. // Validate against the selected stream, not the outer header. @@ -656,7 +667,7 @@ internal partial class ArchiveReader } private void ReadStreamsInfo( - List? dataVector, + List? dataVector, out long dataOffset, out List packSizes, out List packCrCs, @@ -775,8 +786,27 @@ internal partial class ArchiveReader } } - private List ReadAndDecodePackedStreams(long baseOffset, IPasswordProvider pass) + private sealed class DecodedMetadataStreams(int capacity) + : List(capacity), + IDisposable { + public void Dispose() + { + foreach (var stream in this) + { + stream.Dispose(); + } + Clear(); + } + } + + private DecodedMetadataStreams ReadAndDecodePackedStreams( + long baseOffset, + IPasswordProvider pass + ) + { + DecodedMetadataStreams? dataVector = null; + var buffer = decoderBufferPool.Rent(MetadataReadBufferSize); try { ReadStreamsInfo( @@ -792,7 +822,7 @@ internal partial class ArchiveReader dataStartPos += baseOffset; - var dataVector = new List(folders.Count); + dataVector = new DecodedMetadataStreams(folders.Count); var packIndex = 0; foreach (var folder in folders) { @@ -819,28 +849,27 @@ internal partial class ArchiveReader // Do not reserve the declared unpack size: malformed compressed // metadata can advertise GiB of output without producing a byte. - using var decoded = new PooledMemoryStream(); - var buffer = new byte[8192]; + var decoded = new PooledMemoryStream(0, Constants.BufferSize, decoderBufferPool); + dataVector.Add(decoded); + var crc = Crc.INIT_CRC; for (var remaining = unpackSize; remaining > 0; ) { - var count = Math.Min(buffer.Length, remaining); + var count = Math.Min(MetadataReadBufferSize, remaining); outStream.ReadExact(buffer, 0, count); decoded.Write(buffer, 0, count); + if (folder.UnpackCrcDefined) + { + crc = Crc.Update(crc, buffer, 0, count); + } remaining -= count; } if (outStream.ReadByte() >= 0) { throw new InvalidFormatException("Decoded stream is longer than expected."); } - var data = decoded.ToArray(); - dataVector.Add(data); - if (folder.UnpackCrcDefined) { - if ( - Crc.Finish(Crc.Update(Crc.INIT_CRC, data, 0, unpackSize)) - != folder._unpackCrc - ) + if (Crc.Finish(crc) != folder._unpackCrc) { var exception = new InvalidFormatException( "Decoded stream does not match expected CRC." @@ -853,7 +882,15 @@ internal partial class ArchiveReader } return dataVector; } - finally { } + catch + { + dataVector?.Dispose(); + throw; + } + finally + { + decoderBufferPool.Return(buffer); + } } private void ReadHeader(ArchiveDatabase db, IPasswordProvider getTextPassword) @@ -868,13 +905,12 @@ internal partial class ArchiveReader type = ReadId(); } - List? dataVector = null; - if (type == BlockType.AdditionalStreamsInfo) + using var dataVector = + type == BlockType.AdditionalStreamsInfo + ? ReadAndDecodePackedStreams(db._startPositionAfterHeader, getTextPassword) + : null; + if (dataVector is not null) { - dataVector = ReadAndDecodePackedStreams( - db._startPositionAfterHeader, - getTextPassword - ); type = ReadId(); } @@ -955,7 +991,7 @@ internal partial class ArchiveReader case BlockType.Name: using (var streamSwitch = new CStreamSwitch()) { - streamSwitch.Set(this, dataVector ?? []); + streamSwitch.Set(this, dataVector); for (var i = 0; i < db._files.Count; i++) { db._files[i].Name = _currentReader.ReadString(); @@ -1241,18 +1277,18 @@ internal partial class ArchiveReader streamSwitch.Set(this, header); var type = ReadId(); - if (type != BlockType.Header) + if (type is not BlockType.Header and not BlockType.EncodedHeader) { - if (type != BlockType.EncodedHeader) - { - throw new ArchiveOperationException(); - } - - var dataVector = ReadAndDecodePackedStreams( - db._startPositionAfterHeader, - db.PasswordProvider - ); + throw new ArchiveOperationException(); + } + // Keep the pooled metadata alive through parsing, including on errors. + using var dataVector = + type == BlockType.EncodedHeader + ? ReadAndDecodePackedStreams(db._startPositionAfterHeader, db.PasswordProvider) + : null; + if (dataVector is not null) + { // compressed header without content is odd but ok if (dataVector.Count == 0) { diff --git a/src/SharpCompress/Common/SevenZip/CStreamSwitch.cs b/src/SharpCompress/Common/SevenZip/CStreamSwitch.cs index 8b429b19..d485bfba 100644 --- a/src/SharpCompress/Common/SevenZip/CStreamSwitch.cs +++ b/src/SharpCompress/Common/SevenZip/CStreamSwitch.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using SharpCompress.Compressors.LZMA; +using SharpCompress.IO; namespace SharpCompress.Common.SevenZip; @@ -33,7 +34,16 @@ internal struct CStreamSwitch : IDisposable _active = true; } - public void Set(ArchiveReader archive, List dataVector) + public void Set(ArchiveReader archive, PooledMemoryStream data) + { + Dispose(); + _archive = archive; + _archive.AddByteStream(data); + _needRemove = true; + _active = true; + } + + public void Set(ArchiveReader archive, IReadOnlyList? dataVector) { Dispose(); _active = true; @@ -42,13 +52,13 @@ internal struct CStreamSwitch : IDisposable if (external != 0) { var dataIndex = archive.ReadNum(); - if (dataIndex < 0 || dataIndex >= dataVector.Count) + if (dataVector is null || dataIndex < 0 || dataIndex >= dataVector.Count) { throw new ArchiveOperationException(); } _archive = archive; - _archive.AddByteStream(dataVector[dataIndex], 0, dataVector[dataIndex].Length); + _archive.AddByteStream(dataVector[dataIndex]); _needRemove = true; _active = true; } diff --git a/src/SharpCompress/Common/SevenZip/DataReader.cs b/src/SharpCompress/Common/SevenZip/DataReader.cs index 85d59be5..595d0d79 100644 --- a/src/SharpCompress/Common/SevenZip/DataReader.cs +++ b/src/SharpCompress/Common/SevenZip/DataReader.cs @@ -2,6 +2,7 @@ using System.IO; using System.Text; using SharpCompress.Compressors.LZMA; +using SharpCompress.IO; namespace SharpCompress.Common.SevenZip; @@ -29,7 +30,8 @@ internal class DataReader #region Variables - private readonly byte[] _buffer; + private readonly byte[]? _buffer; + private readonly PooledMemoryStream? _pooledBuffer; private readonly int _ending; #endregion @@ -43,11 +45,28 @@ internal class DataReader _ending = offset + length; } + public DataReader(PooledMemoryStream buffer) + { + _pooledBuffer = buffer; + _ending = checked((int)buffer.Length); + } + + private DataReader(DataReader source) + { + _buffer = source._buffer; + _pooledBuffer = source._pooledBuffer; + _ending = source._ending; + Offset = source.Offset; + } + public int Offset { get; private set; } public int Remaining => _ending - Offset; - public DataReader Clone() => new(_buffer, Offset, Remaining); + public DataReader Clone() => new(this); + + private byte GetByte(int offset) => + _buffer is not null ? _buffer[offset] : _pooledBuffer!.ReadByteAt(offset); public void RequireBytes(long count) { @@ -64,16 +83,22 @@ internal class DataReader throw new IncompleteArchiveException("Unexpected end of stream."); } - return _buffer[Offset++]; + return GetByte(Offset++); } public void ReadBytes(byte[] buffer, int offset, int length) { RequireBytes(length); + if (_pooledBuffer is not null) + { + _pooledBuffer.ReadAt(Offset, buffer, offset, length); + Offset += length; + return; + } while (length-- > 0) { - buffer[offset++] = _buffer[Offset++]; + buffer[offset++] = _buffer![Offset++]; } } @@ -93,7 +118,7 @@ internal class DataReader throw new IncompleteArchiveException("Unexpected end of stream."); } - var firstByte = _buffer[Offset++]; + var firstByte = GetByte(Offset++); byte mask = 0x80; ulong value = 0; @@ -111,7 +136,7 @@ internal class DataReader throw new IncompleteArchiveException("Unexpected end of stream."); } - value |= (ulong)_buffer[Offset++] << (8 * i); + value |= (ulong)GetByte(Offset++) << (8 * i); mask >>= 1; } @@ -131,26 +156,36 @@ internal class DataReader public uint ReadUInt32() { - if (Offset + 4 > _ending) + RequireBytes(4); + if (_buffer is not null) { - throw new IncompleteArchiveException("Unexpected end of stream."); + var res = Get32(_buffer, Offset); + Offset += 4; + return res; } - - var res = Get32(_buffer, Offset); - Offset += 4; - return res; + uint value = 0; + for (var i = 0; i < 4; i++) + { + value |= (uint)ReadByte() << (8 * i); + } + return value; } public ulong ReadUInt64() { - if (Offset + 8 > _ending) + RequireBytes(8); + if (_buffer is not null) { - throw new IncompleteArchiveException("Unexpected end of stream."); + var res = Get64(_buffer, Offset); + Offset += 8; + return res; } - - var res = Get64(_buffer, Offset); - Offset += 8; - return res; + ulong value = 0; + for (var i = 0; i < 8; i++) + { + value |= (ulong)ReadByte() << (8 * i); + } + return value; } public string ReadString() @@ -159,12 +194,12 @@ internal class DataReader for (; ; ) { - if (ending + 2 > _ending) + if (_ending - ending < 2) { throw new IncompleteArchiveException("Unexpected end of stream."); } - if (_buffer[ending] == 0 && _buffer[ending + 1] == 0) + if (GetByte(ending) == 0 && GetByte(ending + 1) == 0) { break; } @@ -172,10 +207,35 @@ internal class DataReader ending += 2; } - var str = Encoding.Unicode.GetString(_buffer, Offset, ending - Offset); + var str = _buffer is not null + ? Encoding.Unicode.GetString(_buffer, Offset, ending - Offset) + : ReadPooledString(ending - Offset); Offset = ending + 2; return str; } + private string ReadPooledString(int length) + { + if (length == 0) + { + return string.Empty; + } + // Decode bounded chunks, preserving surrogate pairs across block boundaries, + // rather than copying a potentially header-sized name into a temporary array. + var bytes = new byte[Math.Min(length, 256)]; + var chars = new char[Encoding.Unicode.GetMaxCharCount(bytes.Length)]; + var decoder = Encoding.Unicode.GetDecoder(); + var result = new StringBuilder(); + for (var offset = 0; offset < length; ) + { + var count = Math.Min(bytes.Length, length - offset); + _pooledBuffer!.ReadAt(Offset + offset, bytes, 0, count); + var charCount = decoder.GetChars(bytes, 0, count, chars, 0, offset + count == length); + result.Append(chars, 0, charCount); + offset += count; + } + return result.ToString(); + } + #endregion } diff --git a/src/SharpCompress/IO/PooledMemoryStream.cs b/src/SharpCompress/IO/PooledMemoryStream.cs index 58633a7c..4e6cac24 100644 --- a/src/SharpCompress/IO/PooledMemoryStream.cs +++ b/src/SharpCompress/IO/PooledMemoryStream.cs @@ -631,6 +631,28 @@ public sealed class PooledMemoryStream : MemoryStream } } + // Metadata readers keep independent offsets over the same pooled storage. + internal byte ReadByteAt(int position) + { + EnsureNotClosed(); + if ((uint)position >= (uint)_length) + { + throw new ArgumentOutOfRangeException(nameof(position)); + } + return _blocks![position / _blockSize][position % _blockSize]; + } + + internal void ReadAt(int position, byte[] buffer, int offset, int count) + { + EnsureNotClosed(); + ValidateReadWriteBufferArguments(buffer, offset, count); + if (position < 0 || position > _length || count > _length - position) + { + throw new ArgumentOutOfRangeException(nameof(position)); + } + CopyFromSegmented(position, buffer, offset, count); + } + private void CopyFromSegmented( int absoluteSourcePosition, byte[] destination, diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipDataReaderTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipDataReaderTests.cs new file mode 100644 index 00000000..ae951e62 --- /dev/null +++ b/tests/SharpCompress.Test/SevenZip/SevenZipDataReaderTests.cs @@ -0,0 +1,67 @@ +using System; +using System.IO; +using System.Text; +using SharpCompress.Common; +using SharpCompress.Common.SevenZip; +using SharpCompress.Crypto; +using SharpCompress.IO; +using Xunit; + +namespace SharpCompress.Test.SevenZip; + +public class SevenZipDataReaderTests +{ + [Theory] + [InlineData(false)] + [InlineData(true)] + public void DataReader_Reads_And_Clones_Across_Storage_Boundaries(bool pooled) + { + using var data = new MemoryStream(); + using (var writer = new BinaryWriter(data, Encoding.UTF8, true)) + { + writer.Write(0x12345678u); + writer.Write(0x123456789abcdef0ul); + } + data.WriteEncodedUInt64(ulong.MaxValue); + // The surrogate pair crosses a UTF-16 decoding chunk as well as pooled blocks. + var name = new string('x', 127) + "\U0001f600" + new string('y', 130); + var nameBytes = Encoding.Unicode.GetBytes(name + "\0"); + data.Write(nameBytes, 0, nameBytes.Length); + using var storage = new PooledMemoryStream(0, 7); + var bytes = data.ToArray(); + storage.Write(bytes, 0, bytes.Length); + var reader = pooled ? new DataReader(storage) : new DataReader(bytes, 0, bytes.Length); + Assert.Equal(0x12345678u, reader.ReadUInt32()); + var clone = reader.Clone(); + Assert.Equal(0x123456789abcdef0ul, clone.ReadUInt64()); + Assert.Equal(4, reader.Offset); + Assert.Equal(0x123456789abcdef0ul, reader.ReadUInt64()); + Assert.Equal(ulong.MaxValue, reader.ReadNumber()); + Assert.Equal(ulong.MaxValue, clone.ReadNumber()); + Assert.Equal(name, reader.ReadString()); + Assert.Equal(name, clone.ReadString()); + Assert.Equal(0, reader.Remaining); + Assert.Throws(() => reader.ReadByte()); + Assert.Throws(() => reader.SkipData(1)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void DataReader_Bounds_Bulk_Reads_And_Unterminated_Strings(bool pooled) + { + byte[] bytes = [1, 2, 3, 4, 5]; + using var storage = new PooledMemoryStream(0, 3); + storage.Write(bytes, 0, bytes.Length); + var reader = pooled ? new DataReader(storage) : new DataReader(bytes, 0, bytes.Length); + var output = new byte[4]; + reader.SkipData(1); + reader.ReadBytes(output, 0, output.Length); + Assert.Equal(new byte[] { 2, 3, 4, 5 }, output); + Assert.Throws(() => reader.ReadBytes(output, 0, 1)); + var stringReader = pooled + ? new DataReader(storage) + : new DataReader(bytes, 0, bytes.Length); + Assert.Throws(() => stringReader.ReadString()); + } +} diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipHeaderLifetimeTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderLifetimeTests.cs index f6d6256e..d163a65d 100644 --- a/tests/SharpCompress.Test/SevenZip/SevenZipHeaderLifetimeTests.cs +++ b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderLifetimeTests.cs @@ -11,6 +11,7 @@ using Xunit; namespace SharpCompress.Test.SevenZip; +[Collection(nameof(SevenZipHeaderAllocationTests))] public class SevenZipHeaderLifetimeTests : TestBase { [Theory] @@ -113,14 +114,125 @@ public class SevenZipHeaderLifetimeTests : TestBase Assert.True(input.CanRead); } + [Theory] + [InlineData(false, 0)] + [InlineData(true, 0)] + [InlineData(false, 1)] + [InlineData(true, 1)] + [InlineData(false, 2)] + [InlineData(true, 2)] + [InlineData(true, 3)] + public async Task Decoded_Metadata_Is_Not_Copied_And_Returns_All_Buffers( + bool asyncRead, + int failure + ) + { + using var decoded = new MemoryStream(); + decoded.Write(new byte[] { failure == 1 ? (byte)99 : (byte)1, 2, 26 }, 0, 3); + decoded.WriteEncodedUInt64(1 << 20); + var padding = new byte[1 << 20]; + decoded.Write(padding, 0, padding.Length); + decoded.WriteByte(0); + decoded.WriteByte(0); + using var encoded = new MemoryStream(); + encoded.Write(new byte[] { 23, 6, 0, 1, 9 }, 0, 5); + encoded.WriteEncodedUInt64((ulong)decoded.Length); + encoded.Write(new byte[] { 0, 7, 11, 1, 0, 1, 1, 0, 12 }, 0, 9); + encoded.WriteEncodedUInt64((ulong)decoded.Length + (failure == 2 ? 8192ul : 0)); + encoded.WriteByte(0); + encoded.WriteByte(0); + var header = encoded.ToArray(); + using var input = new MemoryStream(); + SevenZipSignatureHeaderWriter.WriteFinal( + input, + (ulong)decoded.Length, + (ulong)header.Length, + Crc32Stream.Compute(header) + ); + decoded.Position = 0; + decoded.CopyTo(input); + input.Write(header, 0, header.Length); + input.Position = 0; + using var cancellation = new CancellationTokenSource(); + var pool = new TrackingPool + { + OnRent = + failure == 3 + ? count => + { + // Scratch and packed-input buffers are rented before metadata storage. + if (count == 3) + { + cancellation.Cancel(); + } + } + : null, + }; + var reader = new ArchiveReader(pool); + if (asyncRead) + { + await reader.OpenAsync(input, false, CancellationToken.None); + } + else + { + reader.Open(input, false); + } +#if !LEGACY_DOTNET + var before = GC.GetTotalAllocatedBytes(precise: true); +#endif + if (failure == 3) + { + await Assert.ThrowsAnyAsync(async () => + await reader.ReadDatabaseAsync(new NoPassword(), cancellation.Token) + ); + } + else if (failure == 0) + { + var database = asyncRead + ? await reader.ReadDatabaseAsync(new NoPassword(), CancellationToken.None) + : reader.ReadDatabase(new NoPassword()); + Assert.Empty(database._files); + } + else if (asyncRead) + { + await Assert.ThrowsAnyAsync(async () => + await reader.ReadDatabaseAsync(new NoPassword(), CancellationToken.None) + ); + } + else + { + Assert.ThrowsAny(() => reader.ReadDatabase(new NoPassword())); + } +#if !LEGACY_DOTNET + var allocated = GC.GetTotalAllocatedBytes(precise: true) - before; + Assert.True( + allocated < pool.AllocatedBytes + 512 * 1024, + $"Allocated {allocated} bytes including {pool.AllocatedBytes} bytes of pooled storage." + ); +#endif + Assert.True(pool.Rents > 0); + Assert.Equal(1, pool.ScratchRents); + Assert.Equal(pool.Rents, pool.Returns); + Assert.True(input.CanRead); + } + private sealed class TrackingPool : ArrayPool { public int Rents { get; private set; } public int Returns { get; private set; } + public int ScratchRents { get; private set; } + public long AllocatedBytes { get; private set; } + public Action? OnRent { get; set; } public override byte[] Rent(int minimumLength) { Rents++; + if (minimumLength == 8192) + { + ScratchRents++; + } + AllocatedBytes += minimumLength; + OnRent?.Invoke(Rents); return new byte[minimumLength]; } From 2b9295ce36100f2bc1ab93912096ceab709bf3b1 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 10:54:49 +0000 Subject: [PATCH 23/26] Fix GZip header position when resuming partial enumeration --- .../Archives/GZip/GZipArchive.Async.cs | 1 + .../Archives/GZip/GZipArchive.cs | 6 +- .../GZip/GZipArchiveCacheTests.cs | 80 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs diff --git a/src/SharpCompress/Archives/GZip/GZipArchive.Async.cs b/src/SharpCompress/Archives/GZip/GZipArchive.Async.cs index 032e33f3..d59b212b 100644 --- a/src/SharpCompress/Archives/GZip/GZipArchive.Async.cs +++ b/src/SharpCompress/Archives/GZip/GZipArchive.Async.cs @@ -110,6 +110,7 @@ public partial class GZipArchive ) { var stream = (await volumes.SingleAsync().ConfigureAwait(false)).Stream; + stream.Position = headerPosition; yield return new GZipArchiveEntry( this, await GZipFilePart diff --git a/src/SharpCompress/Archives/GZip/GZipArchive.cs b/src/SharpCompress/Archives/GZip/GZipArchive.cs index ab3a009d..72069566 100644 --- a/src/SharpCompress/Archives/GZip/GZipArchive.cs +++ b/src/SharpCompress/Archives/GZip/GZipArchive.cs @@ -15,8 +15,10 @@ namespace SharpCompress.Archives.GZip; public partial class GZipArchive : AbstractWritableArchive { + private readonly long headerPosition; + private GZipArchive(SourceStream sourceStream) - : base(ArchiveType.GZip, sourceStream) { } + : base(ArchiveType.GZip, sourceStream) => headerPosition = sourceStream.Position; internal GZipArchive() : base(ArchiveType.GZip) { } @@ -49,6 +51,8 @@ public partial class GZipArchive { // Kept hand-written: the generator cannot map IAsyncEnumerable.SingleAsync to LINQ Single. var stream = volumes.Single().Stream; + // The other API may already have consumed the header or extracted the entry. + stream.Position = headerPosition; yield return new GZipArchiveEntry( this, GZipFilePart.Create(stream, ReaderOptions.ArchiveEncoding, ReaderOptions.Providers), diff --git a/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs b/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs new file mode 100644 index 00000000..ab6a1715 --- /dev/null +++ b/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs @@ -0,0 +1,80 @@ +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Archives.GZip; +using SharpCompress.Readers; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.GZip; + +public class GZipArchiveCacheTests : TestBase +{ + [Theory] + [InlineData(false, false, false)] + [InlineData(false, true, false)] + [InlineData(true, false, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(false, true, true)] + [InlineData(true, false, true)] + [InlineData(true, true, true)] + public async Task Partial_Enumeration_Can_Be_Resumed_Through_Either_Api( + bool asyncFirst, + bool asyncNext, + bool prefixed + ) + { + using var input = new MemoryStream(); + var start = prefixed ? 17 : 0; + input.Write(new byte[start], 0, start); + using (var fixture = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Tar.tar.gz"))) + { + fixture.CopyTo(input); + } + input.Position = start; + using var archive = GZipArchive.OpenArchive(input, ReaderOptions.ForExternalStream); + var asyncArchive = (IAsyncArchive)archive; + var first = asyncFirst + ? await asyncArchive.EntriesAsync.FirstAsync() + : archive.Entries.First(); + var entries = asyncNext + ? await asyncArchive.EntriesAsync.ToListAsync() + : archive.Entries.ToList(); + Assert.Same(first, Assert.Single(entries)); + Assert.Same(first, Assert.Single(archive.Entries)); + Assert.Same(first, Assert.Single(await asyncArchive.EntriesAsync.ToListAsync())); + using var payload = first.OpenEntryStream(); + Assert.True(payload.ReadByte() >= 0); + Assert.True(input.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Extraction_Does_Not_Invalidate_A_Partially_Consumed_Loader(bool asyncFirst) + { + using var input = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Tar.tar.gz")); + using var archive = GZipArchive.OpenArchive(input, ReaderOptions.ForExternalStream); + var asyncArchive = (IAsyncArchive)archive; + var first = asyncFirst + ? await asyncArchive.EntriesAsync.FirstAsync() + : archive.Entries.First(); + if (asyncFirst) + { + using var payload = await first.OpenEntryStreamAsync(CancellationToken.None); + await payload.CopyToAsync(Stream.Null, 81920, CancellationToken.None); + } + else + { + using var payload = first.OpenEntryStream(); + payload.CopyTo(Stream.Null); + } + var entries = asyncFirst + ? archive.Entries.ToList() + : await asyncArchive.EntriesAsync.ToListAsync(); + Assert.Same(first, Assert.Single(entries)); + } +} From 331808d004bd0720bfbd586f8da1b19a78805e61 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 10:55:26 +0000 Subject: [PATCH 24/26] Use shared archive entry views for completeness checks --- .../Archives/AbstractArchive.Async.cs | 2 +- src/SharpCompress/Archives/AbstractArchive.cs | 3 +- .../GZip/GZipArchiveCacheTests.cs | 72 +++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/Archives/AbstractArchive.Async.cs b/src/SharpCompress/Archives/AbstractArchive.Async.cs index 7c68342f..7a826d2f 100644 --- a/src/SharpCompress/Archives/AbstractArchive.Async.cs +++ b/src/SharpCompress/Archives/AbstractArchive.Async.cs @@ -47,7 +47,7 @@ public abstract partial class AbstractArchive private async ValueTask EnsureEntriesLoadedAsync() { - await _lazyEntriesAsync.EnsureFullyLoaded().ConfigureAwait(false); + await foreach (var entry in EntriesAsync.ConfigureAwait(false)) { } await _lazyVolumesAsync.EnsureFullyLoaded().ConfigureAwait(false); } diff --git a/src/SharpCompress/Archives/AbstractArchive.cs b/src/SharpCompress/Archives/AbstractArchive.cs index 1e4b353b..ac067a27 100644 --- a/src/SharpCompress/Archives/AbstractArchive.cs +++ b/src/SharpCompress/Archives/AbstractArchive.cs @@ -99,7 +99,8 @@ public abstract partial class AbstractArchive : IArchive, IAsyn private void EnsureEntriesLoaded() { - _lazyEntries.EnsureFullyLoaded(); + // Writable archives expose a shared view rather than this base cache. + Entries.ForEach(_ => { }); _lazyVolumes.EnsureFullyLoaded(); } diff --git a/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs b/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs index ab6a1715..650cbfe6 100644 --- a/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs +++ b/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs @@ -51,6 +51,56 @@ public class GZipArchiveCacheTests : TestBase Assert.True(input.CanRead); } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task Completeness_Uses_The_Current_Entry_Cache(bool useAsync, bool partial) + { + using var input = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Tar.tar.gz")); + using var archive = GZipArchive.OpenArchive(input, ReaderOptions.ForExternalStream); + var asyncArchive = (IAsyncArchive)archive; + if (partial) + { + if (useAsync) + { + await asyncArchive.EntriesAsync.FirstAsync(); + } + else + { + _ = archive.Entries.First(); + } + } + Assert.True(useAsync ? await asyncArchive.IsCompleteAsync() : archive.IsComplete); + var entry = Assert.Single(archive.Entries); + Assert.Same(entry, Assert.Single(await asyncArchive.EntriesAsync.ToListAsync())); + Assert.True(archive.IsComplete); + Assert.True(await asyncArchive.IsCompleteAsync()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Completeness_Does_Not_Reparse_Already_Cached_Entries(bool useAsync) + { + using var input = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Tar.tar.gz")); + using var archive = GZipArchive.OpenArchive(input, ReaderOptions.ForExternalStream); + var asyncArchive = (IAsyncArchive)archive; + if (useAsync) + { + Assert.Single(await asyncArchive.EntriesAsync.ToListAsync()); + } + else + { + Assert.Single(archive.Entries); + } + // Completeness needs only cached metadata and must not move the source. + input.Position = input.Length; + Assert.True(useAsync ? await asyncArchive.IsCompleteAsync() : archive.IsComplete); + Assert.Equal(input.Length, input.Position); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -77,4 +127,26 @@ public class GZipArchiveCacheTests : TestBase : await asyncArchive.EntriesAsync.ToListAsync(); Assert.Same(first, Assert.Single(entries)); } + + [Fact] + public async Task Resuming_And_Checking_Completeness_Do_Not_Use_Synchronous_Io() + { + using var input = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Tar.tar.gz")); + using var asyncOnly = new AsyncOnlyStream(input, disposeStream: false); + await using ( + var archive = await GZipArchive.OpenAsyncArchive( + asyncOnly, + ReaderOptions.ForExternalStream, + CancellationToken.None + ) + ) + { + var entry = await archive.EntriesAsync.FirstAsync(); + Assert.True(await archive.IsCompleteAsync()); + Assert.Same(entry, Assert.Single(await archive.EntriesAsync.ToListAsync())); + using var payload = await entry.OpenEntryStreamAsync(CancellationToken.None); + await payload.CopyToAsync(Stream.Null, 81920, CancellationToken.None); + } + Assert.True(input.CanRead); + } } From 4b82e1728effb6851531ba9fac0c5c6db315e04c Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 10:55:55 +0000 Subject: [PATCH 25/26] Avoid quadratic header parsing when switching archive enumeration APIs --- .../Archives/AbstractArchive.Async.cs | 10 +- .../Archives/AbstractWritableArchive.Async.cs | 40 ++-- .../Archives/AbstractWritableArchive.cs | 32 ++-- .../Common/Tar/TarHeaderFactory.Async.cs | 6 + .../Common/Tar/TarHeaderFactory.cs | 7 + .../GZip/GZipArchiveCacheTests.cs | 21 ++ .../SevenZip/SevenZipHeaderAllocationTests.cs | 8 +- .../WritableArchiveIdentityTests.cs | 181 +++++++++++++++++- 8 files changed, 258 insertions(+), 47 deletions(-) diff --git a/src/SharpCompress/Archives/AbstractArchive.Async.cs b/src/SharpCompress/Archives/AbstractArchive.Async.cs index 7a826d2f..cfbb05e8 100644 --- a/src/SharpCompress/Archives/AbstractArchive.Async.cs +++ b/src/SharpCompress/Archives/AbstractArchive.Async.cs @@ -1,5 +1,7 @@ using System.Collections.Generic; using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading; using System.Threading.Tasks; using SharpCompress.Common; using SharpCompress.Readers; @@ -51,9 +53,13 @@ public abstract partial class AbstractArchive await _lazyVolumesAsync.EnsureFullyLoaded().ConfigureAwait(false); } - private async IAsyncEnumerable EntriesAsyncCast() + private async IAsyncEnumerable EntriesAsyncCast( + [EnumeratorCancellation] CancellationToken cancellationToken = default + ) { - await foreach (var entry in EntriesAsync.ConfigureAwait(false)) + await foreach ( + var entry in EntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) + ) { yield return entry; } diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs index 4548a21c..94b6a744 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs @@ -21,12 +21,15 @@ public abstract partial class AbstractWritableArchive [EnumeratorCancellation] CancellationToken cancellationToken = default ) { + cancellationToken.ThrowIfCancellationRequested(); await foreach ( var entry in OldEntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) ) { + cancellationToken.ThrowIfCancellationRequested(); yield return entry; } + cancellationToken.ThrowIfCancellationRequested(); foreach (var entry in newEntries) { cancellationToken.ThrowIfCancellationRequested(); @@ -47,6 +50,7 @@ public abstract partial class AbstractWritableArchive var index = 0; while (true) { + cancellationToken.ThrowIfCancellationRequested(); while (index < originalEntries.Count) { cancellationToken.ThrowIfCancellationRequested(); @@ -57,28 +61,25 @@ public abstract partial class AbstractWritableArchive yield break; } - var generation = ++originalEntriesGeneration; - await using var source = LoadEntriesAsync(VolumesAsync) - .GetAsyncEnumerator(cancellationToken); - for (var skipped = 0; skipped < index; skipped++) + // The cached loader outlives individual enumerations and must not + // retain a caller's cancellation token after that enumeration ends. + originalEntriesAsyncSource ??= base.EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + while (await originalEntriesAsyncSource.MoveNextAsync().ConfigureAwait(false)) { - if (!await source.MoveNextAsync().ConfigureAwait(false)) + if (originalEntriesAsyncRead++ < originalEntries.Count) { - throw new IncompleteArchiveException("Archive entries changed while loading."); + cancellationToken.ThrowIfCancellationRequested(); + continue; } + originalEntries.Add(originalEntriesAsyncSource.Current); + break; } - while ( - generation == originalEntriesGeneration - && await source.MoveNextAsync().ConfigureAwait(false) - ) - { - cancellationToken.ThrowIfCancellationRequested(); - originalEntries.Add(source.Current); - index++; - yield return source.Current; - } - if (generation == originalEntriesGeneration) + if (index == originalEntries.Count) { + await originalEntriesAsyncSource.DisposeAsync().ConfigureAwait(false); + originalEntriesAsyncSource = null; originalEntriesLoaded = true; } } @@ -198,6 +199,11 @@ public abstract partial class AbstractWritableArchive public override async ValueTask DisposeAsync() { await base.DisposeAsync().ConfigureAwait(false); + originalEntriesSource?.Dispose(); + if (originalEntriesAsyncSource is not null) + { + await originalEntriesAsyncSource.DisposeAsync().ConfigureAwait(false); + } originalEntries.Cast().ForEach(x => x.Close()); newEntries.Cast().ForEach(x => x.Close()); removedEntries.Cast().ForEach(x => x.Close()); diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.cs b/src/SharpCompress/Archives/AbstractWritableArchive.cs index dea9de08..3fcc93d0 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.cs @@ -44,7 +44,10 @@ public abstract partial class AbstractWritableArchive private readonly List originalEntries = new(); private readonly LazyReadOnlyCollection originalEntriesView; private bool originalEntriesLoaded; - private int originalEntriesGeneration; + private IEnumerator? originalEntriesSource; + private IAsyncEnumerator? originalEntriesAsyncSource; + private int originalEntriesRead; + private int originalEntriesAsyncRead; private readonly List modifiedEntries = new(); private bool hasModifications; @@ -109,25 +112,23 @@ public abstract partial class AbstractWritableArchive yield break; } - // Both APIs share entry identities. Switching loaders must restart - // parsing: the other loader may have moved the archive's source stream. - var generation = ++originalEntriesGeneration; - using var source = LoadEntries(Volumes).GetEnumerator(); - for (var skipped = 0; skipped < index; skipped++) + // Resume the base cache instead of reparsing its prefix on every API + // switch. Each loader scans at most once; only the first entry object + // seen at each index is exposed to callers and removal checks. + originalEntriesSource ??= base.Entries.GetEnumerator(); + while (originalEntriesSource.MoveNext()) { - if (!source.MoveNext()) + if (originalEntriesRead++ < originalEntries.Count) { - throw new IncompleteArchiveException("Archive entries changed while loading."); + continue; } + originalEntries.Add(originalEntriesSource.Current); + break; } - while (generation == originalEntriesGeneration && source.MoveNext()) - { - originalEntries.Add(source.Current); - index++; - yield return source.Current; - } - if (generation == originalEntriesGeneration) + if (index == originalEntries.Count) { + originalEntriesSource.Dispose(); + originalEntriesSource = null; originalEntriesLoaded = true; } } @@ -306,6 +307,7 @@ public abstract partial class AbstractWritableArchive public override void Dispose() { base.Dispose(); + originalEntriesSource?.Dispose(); originalEntries.Cast().ForEach(x => x.Close()); newEntries.Cast().ForEach(x => x.Close()); removedEntries.Cast().ForEach(x => x.Close()); diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs index d1755ad4..d867398f 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs @@ -21,12 +21,17 @@ internal static partial class TarHeaderFactory #endif var globalPaxMetadata = new TarHeader.PaxMetadata(); + var position = mode == StreamingMode.Seekable ? stream.Position : 0; while (true) { TarHeader? header = null; try { + if (mode == StreamingMode.Seekable) + { + stream.Position = position; + } header = new TarHeader(archiveEncoding); if (!await header.ReadAsync(reader, globalPaxMetadata).ConfigureAwait(false)) { @@ -42,6 +47,7 @@ internal static partial class TarHeaderFactory // Reject padded sizes or end offsets that would wrap into a backward seek. stream.Position = checked(stream.Position + paddedSize); + position = stream.Position; } break; case StreamingMode.Streaming: diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs index e3a19d22..64059d5d 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs @@ -14,11 +14,17 @@ internal static partial class TarHeaderFactory ) { var globalPaxMetadata = new TarHeader.PaxMetadata(); + var position = mode == StreamingMode.Seekable ? stream.Position : 0; while (true) { TarHeader? header = null; try { + if (mode == StreamingMode.Seekable) + { + // Another entry reader or loader may have moved the shared stream. + stream.Position = position; + } var reader = new BinaryReader(stream, archiveEncoding.Default, leaveOpen: false); header = new TarHeader(archiveEncoding); @@ -36,6 +42,7 @@ internal static partial class TarHeaderFactory // Reject padded sizes or end offsets that would wrap into a backward seek. stream.Position = checked(stream.Position + paddedSize); + position = stream.Position; } break; case StreamingMode.Streaming: diff --git a/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs b/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs index 650cbfe6..fe2b16e8 100644 --- a/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs +++ b/tests/SharpCompress.Test/GZip/GZipArchiveCacheTests.cs @@ -149,4 +149,25 @@ public class GZipArchiveCacheTests : TestBase } Assert.True(input.CanRead); } + + [Fact] + public async Task Cancelling_An_Enumeration_Does_Not_Poison_The_Shared_Cache() + { + using var input = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Tar.tar.gz")); + await using var archive = await GZipArchive.OpenAsyncArchive( + input, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + using var cancellation = new CancellationTokenSource(); + await using (var entries = archive.EntriesAsync.GetAsyncEnumerator(cancellation.Token)) + { + Assert.True(await entries.MoveNextAsync()); + cancellation.Cancel(); + await Assert.ThrowsAnyAsync(async () => + await entries.MoveNextAsync() + ); + } + Assert.Single(await archive.EntriesAsync.ToListAsync()); + } } diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs index 90f4dc24..c05c5b9d 100644 --- a/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs +++ b/tests/SharpCompress.Test/SevenZip/SevenZipHeaderAllocationTests.cs @@ -399,14 +399,12 @@ public class SevenZipHeaderAllocationTests if (minorVersion > 2) { await Assert.ThrowsAnyAsync(async () => - await archive.EntriesAsync.ToListAsync(CancellationToken.None) + await archive.EntriesAsync.ToListAsync() ); } else { - var entry = Assert.Single( - await archive.EntriesAsync.ToListAsync(CancellationToken.None) - ); + var entry = Assert.Single(await archive.EntriesAsync.ToListAsync()); Assert.True(entry.IsDirectory); Assert.Equal("x", entry.Key); } @@ -444,7 +442,7 @@ public class SevenZipHeaderAllocationTests cancellationToken: CancellationToken.None ); await Assert.ThrowsAsync(async () => - await archive.EntriesAsync.ToListAsync(CancellationToken.None) + await archive.EntriesAsync.ToListAsync() ); } else diff --git a/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs b/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs index aad0fd86..3bbe2693 100644 --- a/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs +++ b/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs @@ -38,11 +38,11 @@ public class WritableArchiveIdentityTests var asyncWritable = (IWritableAsyncArchive)archive; // Leave the first loader partially consumed before switching APIs. var entry = asyncFirst - ? await asyncWritable.EntriesAsync.FirstAsync(CancellationToken.None) + ? await asyncWritable.EntriesAsync.FirstAsync() : archive.Entries.First(); var otherEntry = asyncFirst ? archive.Entries.First() - : await asyncWritable.EntriesAsync.FirstAsync(CancellationToken.None); + : await asyncWritable.EntriesAsync.FirstAsync(); Assert.Same(entry, otherEntry); using var source = new MemoryStream(new byte[] { 42 }); @@ -65,7 +65,7 @@ public class WritableArchiveIdentityTests } } var syncEntries = archive.Entries.ToList(); - var asyncEntries = await asyncWritable.EntriesAsync.ToListAsync(CancellationToken.None); + var asyncEntries = await asyncWritable.EntriesAsync.ToListAsync(); Assert.Equal(new[] { "keep", "new" }, syncEntries.Select(e => e.Key)); Assert.Equal(syncEntries, asyncEntries); @@ -96,7 +96,7 @@ public class WritableArchiveIdentityTests var asyncArchive = (IWritableAsyncArchive)archive; if (asyncFirst) { - var entry = await asyncArchive.EntriesAsync.FirstAsync(CancellationToken.None); + var entry = await asyncArchive.EntriesAsync.FirstAsync(); ((IWritableArchive)archive).RemoveEntry(entry); } else @@ -104,10 +104,7 @@ public class WritableArchiveIdentityTests await asyncArchive.RemoveEntryAsync(archive.Entries.First()); } var remaining = Assert.Single(archive.Entries); - Assert.Same( - remaining, - Assert.Single(await asyncArchive.EntriesAsync.ToListAsync(CancellationToken.None)) - ); + Assert.Same(remaining, Assert.Single(await asyncArchive.EntriesAsync.ToListAsync())); using var payload = remaining.OpenEntryStream(); Assert.Equal(2, payload.ReadByte()); } @@ -137,6 +134,174 @@ public class WritableArchiveIdentityTests Assert.False(await asyncEntries.MoveNextAsync()); } + [Theory] + [InlineData(ArchiveType.Zip)] + [InlineData(ArchiveType.Tar)] + public async Task Alternating_Loaders_Read_Only_A_Linear_Number_Of_Headers(ArchiveType type) + { + const int count = 128; + var names = Enumerable.Range(0, count).Select(i => $"entry-{i}").ToArray(); + using var data = CreateArchive(type, names); + using var input = new CountingReadStream(data); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + using var syncEntries = archive.Entries.GetEnumerator(); + await using var asyncEntries = ((IAsyncArchive)archive).EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + for (var i = 0; i < count; i++) + { + if ((i & 1) == 0) + { + Assert.True(syncEntries.MoveNext()); + Assert.True(await asyncEntries.MoveNextAsync()); + } + else + { + Assert.True(await asyncEntries.MoveNextAsync()); + Assert.True(syncEntries.MoveNext()); + } + Assert.Same(syncEntries.Current, asyncEntries.Current); + Assert.Equal(names[i], syncEntries.Current.Key); + } + Assert.False(syncEntries.MoveNext()); + Assert.False(await asyncEntries.MoveNextAsync()); + // Allow two scans and ZIP's end-of-directory search, but not a prefix + // reparse for every entry. This bound does not depend on timing. + Assert.True( + input.BytesRead <= input.Length * 6, + $"Read {input.BytesRead} bytes for a {input.Length}-byte archive." + ); + } + + [Theory] + [InlineData(ArchiveType.Zip, false)] + [InlineData(ArchiveType.Zip, true)] + [InlineData(ArchiveType.Tar, false)] + [InlineData(ArchiveType.Tar, true)] + public async Task Partial_Enumerations_Stay_Lazy_And_Reuse_Their_Loader( + ArchiveType type, + bool useAsync + ) + { + var names = Enumerable.Range(0, 16).Select(i => $"entry-{i}").ToArray(); + using var data = CreateArchive(type, names); + using var input = new CountingReadStream(data); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + var asyncArchive = (IAsyncArchive)archive; + var first = useAsync + ? await asyncArchive.EntriesAsync.FirstAsync() + : archive.Entries.First(); + var readsAfterFirst = input.BytesRead; + var repeated = useAsync + ? await asyncArchive.EntriesAsync.FirstAsync() + : archive.Entries.First(); + Assert.Same(first, repeated); + Assert.Equal(readsAfterFirst, input.BytesRead); + var entries = useAsync + ? await asyncArchive.EntriesAsync.ToListAsync() + : archive.Entries.ToList(); + Assert.Equal(names, entries.Select(e => e.Key)); + Assert.True( + input.BytesRead > readsAfterFirst, + "The first request must not load every header." + ); + } + + [Theory] + [InlineData(ArchiveType.Zip)] + [InlineData(ArchiveType.Tar)] + public async Task Cancellation_During_A_Read_Does_Not_Lose_An_Entry(ArchiveType type) + { + using var data = CreateArchive(type, "first", "second", "third"); + using var input = new CountingReadStream(data); + using var cancellation = new CancellationTokenSource(); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + input, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + var first = await archive.EntriesAsync.FirstAsync(); + await using (var entries = archive.EntriesAsync.GetAsyncEnumerator(cancellation.Token)) + { + Assert.True(await entries.MoveNextAsync()); + Assert.Same(first, entries.Current); + input.AfterRead = cancellation.Cancel; + await Assert.ThrowsAnyAsync(async () => + await entries.MoveNextAsync() + ); + } + input.AfterRead = null; + var remaining = await archive.EntriesAsync.ToListAsync(); + Assert.Equal(new[] { "first", "second", "third" }, remaining.Select(e => e.Key)); + Assert.Same(first, remaining[0]); + } + + private sealed class CountingReadStream(Stream source) : Stream + { + public long BytesRead { get; private set; } + public Action? AfterRead { get; set; } + public override bool CanRead => source.CanRead; + public override bool CanSeek => source.CanSeek; + public override bool CanWrite => false; + public override long Length => source.Length; + public override long Position + { + get => source.Position; + set => source.Position = value; + } + + public override int Read(byte[] buffer, int offset, int count) + { + var read = source.Read(buffer, offset, count); + BytesRead += read; + AfterRead?.Invoke(); + return read; + } + + public override async Task ReadAsync( + byte[] buffer, + int offset, + int count, + CancellationToken cancellationToken + ) + { + var read = await source.ReadAsync(buffer, offset, count, cancellationToken); + BytesRead += read; + AfterRead?.Invoke(); + return read; + } + +#if !LEGACY_DOTNET + public override int Read(Span buffer) + { + var read = source.Read(buffer); + BytesRead += read; + AfterRead?.Invoke(); + return read; + } + + public override async ValueTask ReadAsync( + Memory buffer, + CancellationToken cancellationToken = default + ) + { + var read = await source.ReadAsync(buffer, cancellationToken); + BytesRead += read; + AfterRead?.Invoke(); + return read; + } +#endif + + public override long Seek(long offset, SeekOrigin origin) => source.Seek(offset, origin); + + public override void Flush() => source.Flush(); + + public override void SetLength(long value) => throw new NotSupportedException(); + + public override void Write(byte[] buffer, int offset, int count) => + throw new NotSupportedException(); + } + private static MemoryStream CreateArchive(ArchiveType type, params string[] names) { var input = new MemoryStream(); From e676a26f0596746af2aed55ccd69b2b080b37333 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 13 Nov 2026 11:39:14 +0000 Subject: [PATCH 26/26] Remove openentrystream checks --- .../SevenZip/SevenZipDeflateSolidTests.cs | 4 ---- tests/SharpCompress.Test/Zip/Zip64DescriptorSizeTests.cs | 8 -------- tests/SharpCompress.Test/Zip/Zip64NonSeekableTests.cs | 4 ---- tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs | 4 ---- 4 files changed, 20 deletions(-) diff --git a/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs b/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs index 085abf82..328c0f21 100644 --- a/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs +++ b/tests/SharpCompress.Test/SevenZip/SevenZipDeflateSolidTests.cs @@ -102,11 +102,7 @@ public class SevenZipDeflateSolidTests : TestBase continue; } -#if LEGACY_DOTNET - using var stream = await reader.OpenEntryStreamAsync(CancellationToken.None); -#else await using var stream = await reader.OpenEntryStreamAsync(CancellationToken.None); -#endif if (index == 0 && firstEntryMode == 1) { Assert.True( diff --git a/tests/SharpCompress.Test/Zip/Zip64DescriptorSizeTests.cs b/tests/SharpCompress.Test/Zip/Zip64DescriptorSizeTests.cs index f4aeb6ce..6dc0e132 100644 --- a/tests/SharpCompress.Test/Zip/Zip64DescriptorSizeTests.cs +++ b/tests/SharpCompress.Test/Zip/Zip64DescriptorSizeTests.cs @@ -363,11 +363,7 @@ public class Zip64DescriptorSizeTests ); Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); var first = reader.Entry; -#if LEGACY_DOTNET - using (var stream = await reader.OpenEntryStreamAsync(CancellationToken.None)) -#else await using (var stream = await reader.OpenEntryStreamAsync(CancellationToken.None)) -#endif { await stream.CopyToAsync(Stream.Null, 81920, CancellationToken.None); } @@ -378,11 +374,7 @@ public class Zip64DescriptorSizeTests } Assert.Equal("second", reader.Entry.Key); using var second = new MemoryStream(); -#if LEGACY_DOTNET - using (var stream = await reader.OpenEntryStreamAsync(CancellationToken.None)) -#else await using (var stream = await reader.OpenEntryStreamAsync(CancellationToken.None)) -#endif { await stream.CopyToAsync(second, 81920, CancellationToken.None); } diff --git a/tests/SharpCompress.Test/Zip/Zip64NonSeekableTests.cs b/tests/SharpCompress.Test/Zip/Zip64NonSeekableTests.cs index 91f9126b..9a42c7e5 100644 --- a/tests/SharpCompress.Test/Zip/Zip64NonSeekableTests.cs +++ b/tests/SharpCompress.Test/Zip/Zip64NonSeekableTests.cs @@ -143,11 +143,7 @@ public class Zip64NonSeekableTests continue; } Assert.Equal(entries[index].Name, reader.Entry.Key); -#if LEGACY_DOTNET - using var source = await reader.OpenEntryStreamAsync(CancellationToken.None); -#else await using var source = await reader.OpenEntryStreamAsync(CancellationToken.None); -#endif using var extracted = new MemoryStream(); await source.CopyToAsync(extracted, 81920, CancellationToken.None); Assert.Equal(entries[index++].Content, extracted.ToArray()); diff --git a/tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs b/tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs index d9990ec7..9b149f9f 100644 --- a/tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs +++ b/tests/SharpCompress.Test/Zip/ZipBZip2CorruptionTests.cs @@ -77,11 +77,7 @@ public class ZipBZip2CorruptionTests : TestBase { while (await reader.MoveToNextEntryAsync(CancellationToken.None)) { -#if LEGACY_DOTNET - using var entry = await reader.OpenEntryStreamAsync(CancellationToken.None); -#else await using var entry = await reader.OpenEntryStreamAsync(CancellationToken.None); -#endif if (!skipEntry) { await entry.CopyToAsync(Stream.Null, 81920, CancellationToken.None);