[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Alternative nSelectors patch (Was: bzip2 1.0.7 released)



Hi,

On Fri, 2019-06-28 at 13:10 +0200, Mark Wielaard wrote:
> > It seems to me to be important to now split BZ_MAX_SELECTORS into these two
> > parts so as to make it clear to everybody that we're accepting (decompressing)
> > a slightly larger set of inputs than we create (a la that old saying about
> > network protocol implementations), so as to tolerate other compressors.
> 
> That seems good. The attached patch does this and makes it possible to
> decode the problematic bz2 file.

Sorry, it is a bit too late here to properly document this patch and
explain why I think it is a better one than the "split-max-selectors"
fix. But hopefully the new testsuite example and the comment in the
patch make clear what my thinking is.

This resolved both the issue with the large file reported as with the
new test suite file (lbzip2/32767.bz2). The whole testsuite passes now,
even under valgrind and with gcc -fsanitize=undefined.

Comments on the patch idea more than welcome.

Thanks,

Mark
diff --git a/compress.c b/compress.c
index 237620d..76adee6 100644
--- a/compress.c
+++ b/compress.c
@@ -454,7 +454,7 @@ void sendMTFValues ( EState* s )
 
    AssertH( nGroups < 8, 3002 );
    AssertH( nSelectors < 32768 &&
-            nSelectors <= (2 + (900000 / BZ_G_SIZE)),
+            nSelectors <= BZ_MAX_SELECTORS,
             3003 );
 
 
diff --git a/decompress.c b/decompress.c
index 20ce493..3303499 100644
--- a/decompress.c
+++ b/decompress.c
@@ -287,7 +287,7 @@ Int32 BZ2_decompress ( DState* s )
       GET_BITS(BZ_X_SELECTOR_1, nGroups, 3);
       if (nGroups < 2 || nGroups > BZ_N_GROUPS) RETURN(BZ_DATA_ERROR);
       GET_BITS(BZ_X_SELECTOR_2, nSelectors, 15);
-      if (nSelectors < 1 || nSelectors > BZ_MAX_SELECTORS) RETURN(BZ_DATA_ERROR);
+      if (nSelectors < 1) RETURN(BZ_DATA_ERROR);
       for (i = 0; i < nSelectors; i++) {
          j = 0;
          while (True) {
@@ -296,8 +296,14 @@ Int32 BZ2_decompress ( DState* s )
             j++;
             if (j >= nGroups) RETURN(BZ_DATA_ERROR);
          }
-         s->selectorMtf[i] = j;
+         /* Having more than BZ_MAX_SELECTORS doesn't make much sense
+            since they will never be used, but some implementations might
+            "round up" the number of selectors, so just ignore those. */
+         if (i < BZ_MAX_SELECTORS)
+           s->selectorMtf[i] = j;
       }
+      if (nSelectors > BZ_MAX_SELECTORS)
+        nSelectors = BZ_MAX_SELECTORS;
 
       /*--- Undo the MTF values for the selectors. ---*/
       {