The Trunk: Compression-nice.52.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Compression-nice.52.mcz

commits-2
Nicolas Cellier uploaded a new version of Compression to project The Trunk:
http://source.squeak.org/trunk/Compression-nice.52.mcz

==================== Summary ====================

Name: Compression-nice.52
Author: nice
Time: 1 April 2017, 12:30:20.191934 am
UUID: 470505e6-d35d-450a-9804-29b904c5853d
Ancestors: Compression-ul.51

Correct a bug that can cause ZlibPlugin to read past the zipCollection boundary in DeflateBlock.

If the plugin is disabled, there will be a 'subscript is out of bounds: 65537' in DeflateStream fallback code.

ByteString(Object)>>error:
ByteString(Object)>>errorSubscriptBounds:
ByteString(Object)>>at:
ByteString>>at:
ByteString>>byteAt:
GZipWriteStream(DeflateStream)>>updateHashAt:
GZipWriteStream(DeflateStream)>>insertStringAt:
GZipWriteStream(DeflateStream)>>deflateBlock:chainLength:goodMatch:
GZipWriteStream(ZipWriteStream)>>deflateBlock:chainLength:goodMatch:
GZipWriteStream(DeflateStream)>>deflateBlock
GZipWriteStream(DeflateStream)>>next:putAll:startingAt:
GZipWriteStream(DeflateStream)>>nextPutAll:
ByteString(String)>>zipped

This bug is triggered by some input data, for example if you extract the snapshot.bin file from the zip archive the archive http://source.squeak.org/VMMaker/VMMaker.oscog-nice.794.mcz

We need MinMatch bytes to insert the string following the best match.
(confirmed by comment near the end of a C version http://git.savannah.gnu.org/cgit/gzip.git/tree/deflate.c)

Since we pass lastIndex-1 to the primitive, and that the longest match can be up to MaxMatch, we are going to read the zipCollection up to:

        lastIndex - 1 + MaxMatch - 1 + MinMatch - 1 (0 based)

Thus we must have:

        lastIndex - 1 + MaxMatch - 1 + MinMatch - 1 < writeLimit

We have MinMatch = 3, thus the simplification

        lastIndex + MaxMatch < writeLimit

Thus the requirement:

        lastIndex + MaxMatch + 1 <= writeLimit

=============== Diff against Compression-ul.51 ===============

Item was changed:
  ----- Method: DeflateStream>>deflateBlock (in category 'deflating') -----
  deflateBlock
  "Deflate the current contents of the stream"
  | flushNeeded lastIndex |
  (blockStart == nil) ifTrue:[
  "One time initialization for the first block"
  1 to: MinMatch-1 do:[:i| self updateHashAt: i].
  blockStart := 0].
 
  [blockPosition < position] whileTrue:[
+ (position + MaxMatch + 1 > writeLimit)
+ ifTrue:[lastIndex := writeLimit - MaxMatch - 1]
- (position + MaxMatch > writeLimit)
- ifTrue:[lastIndex := writeLimit - MaxMatch]
  ifFalse:[lastIndex := position].
  flushNeeded := self deflateBlock: lastIndex-1
  chainLength: self hashChainLength
  goodMatch: self goodMatchLength.
  flushNeeded ifTrue:[
  self flushBlock.
  blockStart := blockPosition].
  "Make room for more data"
  self moveContentsToFront].
  !


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compression-nice.52.mcz

Tobias Pape
\o/

Thanks!

> On 01.04.2017, at 00:30, [hidden email] wrote:
>
> Nicolas Cellier uploaded a new version of Compression to project The Trunk:
> http://source.squeak.org/trunk/Compression-nice.52.mcz
>
> ==================== Summary ====================
>
> Name: Compression-nice.52
> Author: nice
> Time: 1 April 2017, 12:30:20.191934 am
> UUID: 470505e6-d35d-450a-9804-29b904c5853d
> Ancestors: Compression-ul.51
>
> Correct a bug that can cause ZlibPlugin to read past the zipCollection boundary in DeflateBlock.
>
> If the plugin is disabled, there will be a 'subscript is out of bounds: 65537' in DeflateStream fallback code.
>
> ByteString(Object)>>error:
> ByteString(Object)>>errorSubscriptBounds:
> ByteString(Object)>>at:
> ByteString>>at:
> ByteString>>byteAt:
> GZipWriteStream(DeflateStream)>>updateHashAt:
> GZipWriteStream(DeflateStream)>>insertStringAt:
> GZipWriteStream(DeflateStream)>>deflateBlock:chainLength:goodMatch:
> GZipWriteStream(ZipWriteStream)>>deflateBlock:chainLength:goodMatch:
> GZipWriteStream(DeflateStream)>>deflateBlock
> GZipWriteStream(DeflateStream)>>next:putAll:startingAt:
> GZipWriteStream(DeflateStream)>>nextPutAll:
> ByteString(String)>>zipped
>
> This bug is triggered by some input data, for example if you extract the snapshot.bin file from the zip archive the archive http://source.squeak.org/VMMaker/VMMaker.oscog-nice.794.mcz
>
> We need MinMatch bytes to insert the string following the best match.
> (confirmed by comment near the end of a C version http://git.savannah.gnu.org/cgit/gzip.git/tree/deflate.c)
>
> Since we pass lastIndex-1 to the primitive, and that the longest match can be up to MaxMatch, we are going to read the zipCollection up to:
>
> lastIndex - 1 + MaxMatch - 1 + MinMatch - 1 (0 based)
>
> Thus we must have:
>
> lastIndex - 1 + MaxMatch - 1 + MinMatch - 1 < writeLimit
>
> We have MinMatch = 3, thus the simplification
>
> lastIndex + MaxMatch < writeLimit
>
> Thus the requirement:
>
> lastIndex + MaxMatch + 1 <= writeLimit
>
> =============== Diff against Compression-ul.51 ===============
>
> Item was changed:
>  ----- Method: DeflateStream>>deflateBlock (in category 'deflating') -----
>  deflateBlock
>   "Deflate the current contents of the stream"
>   | flushNeeded lastIndex |
>   (blockStart == nil) ifTrue:[
>   "One time initialization for the first block"
>   1 to: MinMatch-1 do:[:i| self updateHashAt: i].
>   blockStart := 0].
>
>   [blockPosition < position] whileTrue:[
> + (position + MaxMatch + 1 > writeLimit)
> + ifTrue:[lastIndex := writeLimit - MaxMatch - 1]
> - (position + MaxMatch > writeLimit)
> - ifTrue:[lastIndex := writeLimit - MaxMatch]
>   ifFalse:[lastIndex := position].
>   flushNeeded := self deflateBlock: lastIndex-1
>   chainLength: self hashChainLength
>   goodMatch: self goodMatchLength.
>   flushNeeded ifTrue:[
>   self flushBlock.
>   blockStart := blockPosition].
>   "Make room for more data"
>   self moveContentsToFront].
>  !
>
>