 |
 |
@simonmichal commented on this pull request.
In src/XrdCl/XrdClZipArchive.cc:
> @@ -430,6 +430,19 @@ namespace XrdCl
else
std::tie( cdvec, cdmap ) = CDFH::Parse( buff, eocd->cdSize, eocd->nbCdRec );
log->Dump( ZipMsg, "[0x%x] CD records parsed.", this );
+ uint64_t sumSize = 0;
My understanding is you are summing the "compressed" and "uncompressed" size and making sure this does not exceed the archive size, in order to make sure we are not reading past the end of the buffer. Now the buffer will contain either compressed data (in case the archive was compressed) or uncompressed data (in case it was not compressed), so it doesn't make sense to check both. If it is compressed check only the sum of "compressed" file sizes, and if it is uncompressed the sum of "uncompressed" file sizes. In practice (AFAIR), if the file is NOT compressed the value of "compressed" size and "uncompressed" size will be the same (please double check in the documentation: https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT), so in the end you can get away with always only checking "compressed" size.
In src/XrdCl/XrdClZipArchive.cc:
> @@ -430,6 +430,19 @@ namespace XrdCl
else
std::tie( cdvec, cdmap ) = CDFH::Parse( buff, eocd->cdSize, eocd->nbCdRec );
log->Dump( ZipMsg, "[0x%x] CD records parsed.", this );
+ uint64_t sumSize = 0;
+ uint64_t sumCompSize = 0;
+ for (auto it = cdvec.begin(); it != cdvec.end(); it++) {
+ sumSize += (*it)->uncompressedSize;
+ sumCompSize += (*it)->compressedSize;
+ if ((*it)->offset > archsize || (*it)->offset + (*it)->compressedSize > archsize
+ || (*it)->offset + (*it)->uncompressedSize > archsize)
Similarly as in the previous comment, you should only check the "compressed" size.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1