From 77366a67a2120b1028fa04ce762273623d5a3513 Mon Sep 17 00:00:00 2001 From: scottoneil-ms <145607305+scottoneil-ms@users.noreply.github.com> Date: Tue, 23 Jan 2024 18:35:58 -0800 Subject: [PATCH] Fix unsupported exception when checking the Length property on Streams created by ZipArchiveArtifact (#2767) * fixen * release notes --- ReleaseHistory.md | 1 + src/Sarif/PeekableStream.cs | 1 - src/Sarif/ZipArchiveArtifact.cs | 37 +++++++++++++++++++++++++++------ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/ReleaseHistory.md b/ReleaseHistory.md index f755dc0ee..457967a84 100644 --- a/ReleaseHistory.md +++ b/ReleaseHistory.md @@ -6,6 +6,7 @@ * DEP: Update `Microsoft.Data.SqlClient` reference from 2.1.2 to 2.1.7 in `WorkItems` and `Sarif.Multitool.Library` to resolve [CVE-2024-0056](https://github.com/advisories/GHSA-98g6-xh36-x2p7). * DEP: Update `System.Data.SqlClient` reference from 4.8.5 to 4.8.6 in `WorkItems` to resolve [CVE-2024-0056](https://github.com/advisories/GHSA-98g6-xh36-x2p7). * BUG: Update `Stack.Create` method to populate missing `PhysicalLocation` instances when stack frames reference relative file paths. +* BUG: Fix `UnsupportedOperationException` in `ZipArchiveArtifact`. * NEW: Add `IsBinary` property to `IEnumeratedArtifact` and implement the property in `ZipArchiveArtifact`. * PRF: Change default `max-file-size-in-kb` parameter to 10 megabytes. * PRF: Add support for efficiently peeking into non-seekable streams for binary/text categorization. diff --git a/src/Sarif/PeekableStream.cs b/src/Sarif/PeekableStream.cs index 8b67277be..978da13a4 100644 --- a/src/Sarif/PeekableStream.cs +++ b/src/Sarif/PeekableStream.cs @@ -52,7 +52,6 @@ public override int Read(byte[] buffer, int offset, int count) return underlyingStream.Read(buffer, offset, count); } - // This code relies upon delivering partial reads for correctness. Let's say the caller knows the stream length is 1 MB, // and we have a 1 KB peekability window/buffer. If they do a read with count=1000000, we will return a 1 KB read. // Afterwards, cursor will be exactly equal to buffer.Length. A second count=1MB Read() would detect that, transition diff --git a/src/Sarif/ZipArchiveArtifact.cs b/src/Sarif/ZipArchiveArtifact.cs index b2657746e..26b9da28e 100644 --- a/src/Sarif/ZipArchiveArtifact.cs +++ b/src/Sarif/ZipArchiveArtifact.cs @@ -94,8 +94,24 @@ public byte[] Bytes string extension = Path.GetExtension(Uri.ToString()); if (this.binaryExtensions.Contains(extension)) { - this.bytes = new byte[Stream.Length]; - Stream.Read(this.bytes, 0, this.bytes.Length); + // The underlying System.IO.Compression.DeflateStream throws on reads to get_Length. + using var ms = new MemoryStream((int)SizeInBytes.Value); + this.Stream.CopyTo(ms); + + byte[] memStreamBuffer = ms.GetBuffer(); + if (memStreamBuffer.Length == ms.Position) + { + // We might have succeeded in exactly sizing the MemoryStream. In that case, we can just use it. + this.bytes = memStreamBuffer; + } + else + { + // No luck. Have to take a copy to align the buffers. + ms.Position = 0; + this.bytes = new byte[ms.Length]; + + ms.Read(this.bytes, 0, this.bytes.Length); + } } else { @@ -113,18 +129,27 @@ public byte[] Bytes { get { + if (this.contents != null) + { + return this.contents.Length; + } + + if (this.bytes != null) + { + return this.bytes.Length; + } + lock (this.archive) { if (this.entry != null) { return this.entry.Length; } - - return this.contents != null - ? this.contents.Length - : this.bytes.Length; } + + return null; } + set => throw new NotImplementedException(); } }