Limit zip file entries to 100MB by default - #481
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #481 +/- ##
==========================================
- Coverage 73.53% 73.51% -0.03%
==========================================
Files 449 451 +2
Lines 37633 37692 +59
Branches 5174 5183 +9
==========================================
+ Hits 27673 27708 +35
- Misses 8824 8845 +21
- Partials 1136 1139 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit reviewed 6 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on pmachapman).
src/SIL.Machine/Utils/ZipArchiveEntryExtensions.cs line 40 at r1 (raw file):
} return new BoundedStream(entry.Open(), maxUncompressedSize);
Is the BoundedStream class necessary if we are already checking the uncompressed size? Is ZipArchiveEntry.Length not reliable?
pmachapman
left a comment
There was a problem hiding this comment.
@pmachapman made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ddaspit).
src/SIL.Machine/Utils/ZipArchiveEntryExtensions.cs line 40 at r1 (raw file):
Previously, ddaspit (Damien Daspit) wrote…
Is the
BoundedStreamclass necessary if we are already checking the uncompressed size? IsZipArchiveEntry.Lengthnot reliable?
entry.Length is from the zip file entry header. In a corrupted or malicious zip file, this can be inaccurate, as it is set when the zip file entry is created, rather than calculated from the compressed data. As far as I am aware dotnet performs no bounds checking on the stream in its ZipArchiveEntry implementation.
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit made 1 comment and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on pmachapman).
Part of the fix for sillsdev/serval#974
This change is