CVE-2016-9841: Insufficient Information (NVD-CWE-noinfo)
value: inffast.c in zlib 1.2.8 might allow context-dependent attackers to have unspecified impact by leveraging improper pointer arithmetic.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 9.8
Published: 2017-05-23T04:29Z
Repository: https://github.com/madler/zlib
Language: C
Files changed: inffast.c
Commit date: 2016-09-21 22:51:15 -0700
Commit message: Use post-increment only in inffast.c.

An old inffast.c optimization turns out to not be optimal anymore
with modern compilers, and furthermore was not compliant with the
C standard, for which decrementing a pointer before its allocated
memory is undefined. Per the recommendation of a security audit of
the zlib code by Trail of Bits and TrustInSoft, in support of the
Mozilla Foundation, this "optimization" was removed, in order to
avoid the possibility of undefined behavior.

Diff: diff --git a/inffast.c b/inffast.c
index bda59ce..f0d163d 100644
--- a/inffast.c
+++ b/inffast.c
@@ -8,29 +8,10 @@
 #include "inflate.h"
 #include "inffast.h"
 
 #ifndef ASMINF
 
-/* Allow machine dependent optimization for post-increment or pre-increment.
-   Based on testing to date,
-   Pre-increment preferred for:
-   - PowerPC G3 (Adler)
-   - MIPS R5000 (Randers-Pehrson)
-   Post-increment preferred for:
-   - none
-   No measurable difference:
-   - Pentium III (Anderson)
-   - M68060 (Nikl)
- */
-#ifdef POSTINC
-#  define OFF 0
-#  define PUP(a) *(a)++
-#else
-#  define OFF 1
-#  define PUP(a) *++(a)
-#endif
-
 /*
    Decode literal, length, and distance codes and write out the resulting
    literal and match bytes until either not enough input or output is
    available, an end-of-block is encountered, or a data error is encountered.
    When large enough input and output buffers are supplied to inflate(), for
@@ -94,13 +75,13 @@ unsigned start;         /* inflate()'s starting value for strm->avail_out */
     unsigned dist;              /* match distance */
     unsigned char FAR *from;    /* where to copy match from */
 
     /* copy state to local variables */
     state = (struct inflate_state FAR *)strm->state;
-    in = strm->next_in - OFF;
+    in = strm->next_in;
     last = in + (strm->avail_in - 5);
-    out = strm->next_out - OFF;
+    out = strm->next_out;
     beg = out - (start - strm->avail_out);
     end = out + (strm->avail_out - 257);
 #ifdef INFLATE_STRICT
     dmax = state->dmax;
 #endif
@@ -117,13 +98,13 @@ unsigned start;         /* inflate()'s starting value for strm->avail_out */
 
     /* decode literals and length/distances until end-of-block or not enough
        input data or output space */
     do {
         if (bits < 15) {
-            hold += (unsigned long)(PUP(in)) << bits;
+            hold += (unsigned long)(*in++) << bits;
             bits += 8;
-            hold += (unsigned long)(PUP(in)) << bits;
+            hold += (unsigned long)(*in++) << bits;
             bits += 8;
         }
         here = lcode[hold & lmask];
       dolen:
         op = (unsigned)(here.bits);
@@ -132,29 +113,29 @@ unsigned start;         /* inflate()'s starting value for strm->avail_out */
         op = (unsigned)(here.op);
         if (op == 0) {                          /* literal */
             Tracevv((stderr, here.val >= 0x20 && here.val < 0x7f ?
                     "inflate:         literal '%c'\n" :
                     "inflate:         literal 0x%02x\n", here.val));
-            PUP(out) = (unsigned char)(here.val);
+            *out++ = (unsigned char)(here.val);
         }
         else if (op & 16) {                     /* length base */
             len = (unsigned)(here.val);
             op &= 15;                           /* number of extra bits */
             if (op) {
                 if (bits < op) {
-                    hold += (unsigned long)(PUP(in)) << bits;
+                    hold += (unsigned long)(*in++) << bits;
                     bits += 8;
                 }
                 len += (unsigned)hold & ((1U << op) - 1);
                 hold >>= op;
                 bits -= op;
             }
             Tracevv((stderr, "inflate:         length %u\n", len));
             if (bits < 15) {
-                hold += (unsigned long)(PUP(in)) << bits;
+                hold += (unsigned long)(*in++) << bits;
                 bits += 8;
-                hold += (unsigned long)(PUP(in)) << bits;
+                hold += (unsigned long)(*in++) << bits;
                 bits += 8;
             }
             here = dcode[hold & dmask];
           dodist:
             op = (unsigned)(here.bits);
@@ -163,14 +144,14 @@ unsigned start;         /* inflate()'s starting value for strm->avail_out */
             op = (unsigned)(here.op);
             if (op & 16) {                      /* distance base */
                 dist = (unsigned)(here.val);
                 op &= 15;                       /* number of extra bits */
                 if (bits < op) {
-                    hold += (unsigned long)(PUP(in)) << bits;
+                    hold += (unsigned long)(*in++) << bits;
                     bits += 8;
                     if (bits < op) {
-                        hold += (unsigned long)(PUP(in)) << bits;
+                        hold += (unsigned long)(*in++) << bits;
                         bits += 8;
                     }
                 }
                 dist += (unsigned)hold & ((1U << op) - 1);
 #ifdef INFLATE_STRICT
@@ -194,91 +175,91 @@ unsigned start;         /* inflate()'s starting value for strm->avail_out */
                             break;
                         }
 #ifdef INFLATE_ALLOW_INVALID_DISTANCE_TOOFAR_ARRR
                         if (len <= op - whave) {
                             do {
-                                PUP(out) = 0;
+                                *out++ = 0;
                             } while (--len);
                             continue;
                         }
                         len -= op - whave;
                         do {
-                            PUP(out) = 0;
+                            *out++ = 0;
                         } while (--op > whave);
                         if (op == 0) {
                             from = out - dist;
                             do {
-                                PUP(out) = PUP(from);
+                                *out++ = *from++;
                             } while (--len);
                             continue;
                         }
 #endif
                     }
-                    from = window - OFF;
+                    from = window;
                     if (wnext == 0) {           /* very common case */
                         from += wsize - op;
                         if (op < len) {         /* some from window */
                             len -= op;
                             do {
-                                PUP(out) = PUP(from);
+                                *out++ = *from++;
                             } while (--op);
                             from = out - dist;  /* rest from output */
                         }
                     }
                     else if (wnext < op) {      /* wrap around window */
                         from += wsize + wnext - op;
                         op -= wnext;
                         if (op < len) {         /* some from end of window */
                             len -= op;
                             do {
-                                PUP(out) = PUP(from);
+                                *out++ = *from++;
                             } while (--op);
-                            from = window - OFF;
+                            from = window;
                             if (wnext < len) {  /* some from start of window */
                                 op = wnext;
                                 len -= op;
                                 do {
-                                    PUP(out) = PUP(from);
+                                    *out++ = *from++;
                                 } while (--op);
                                 from = out - dist;      /* rest from output */
                             }
                         }
                     }
                     else {                      /* contiguous in window */
                         from += wnext - op;
                         if (op < len) {         /* some from window */
                             len -= op;
                             do {
-                                PUP(out) = PUP(from);
+                                *out++ = *from++;
                             } while (--op);
                             from = out - dist;  /* rest from output */
                         }
                     }
                     while (len > 2) {
-                        PUP(out) = PUP(from);
-                        PUP(out) = PUP(from);
-                        PUP(out) = PUP(from);
+                        *out++ = *from++;
+                        *out++ = *from++;
+                        *out++ = *from++;
                         len -= 3;
                     }
                     if (len) {
-                        PUP(out) = PUP(from);
+                        *out++ = *from++;
                         if (len > 1)
-                            PUP(out) = PUP(from);
+                            *out++ = *from++;
                     }
                 }
                 else {
                     from = out - dist;          /* copy direct from output */
                     do {                        /* minimum length is three */
-                        PUP(out) = PUP(from);
-                        PUP(out) = PUP(from);
-                        PUP(out) = PUP(from);
+                        *out++ = *from++;
+                        *out++ = *from++;
+                        *out++ = *from++;
                         len -= 3;
                     } while (len > 2);
                     if (len) {
-                        PUP(out) = PUP(from);
+                        *out++ = *from++;
                         if (len > 1)
-                            PUP(out) = PUP(from);
+                            *out++ = *from++;
                     }
                 }
             }
             else if ((op & 64) == 0) {          /* 2nd level distance code */
                 here = dcode[here.val + (hold & ((1U << op) - 1))];
@@ -311,12 +292,12 @@ unsigned start;         /* inflate()'s starting value for strm->avail_out */
     in -= len;
     bits -= len << 3;
     hold &= (1U << bits) - 1;
 
     /* update state and return */
-    strm->next_in = in + OFF;
-    strm->next_out = out + OFF;
+    strm->next_in = in;
+    strm->next_out = out;
     strm->avail_in = (unsigned)(in < last ? 5 + (last - in) : 5 - (in - last));
     strm->avail_out = (unsigned)(out < end ?
                                  257 + (end - out) : 257 - (out - end));
     state->hold = hold;
     state->bits = bits;

Vulnerable code: /* Allow machine dependent optimization for post-increment or pre-increment.
   Based on testing to date,
   Pre-increment preferred for:
   - PowerPC G3 (Adler)
   - MIPS R5000 (Randers-Pehrson)
   Post-increment preferred for:
   - none
   No measurable difference:
   - Pentium III (Anderson)
   - M68060 (Nikl)
 */
#ifdef POSTINC
#  define OFF 0
#  define PUP(a) *(a)++
#else
#  define OFF 1
#  define PUP(a) *++(a)
#endif

    in = strm->next_in - OFF;
    out = strm->next_out - OFF;
            hold += (unsigned long)(PUP(in)) << bits;
            hold += (unsigned long)(PUP(in)) << bits;
            PUP(out) = (unsigned char)(here.val);
                    hold += (unsigned long)(PUP(in)) << bits;
                hold += (unsigned long)(PUP(in)) << bits;
                hold += (unsigned long)(PUP(in)) << bits;
                    hold += (unsigned long)(PUP(in)) << bits;
                        hold += (unsigned long)(PUP(in)) << bits;
                                PUP(out) = 0;
                            PUP(out) = 0;
                                PUP(out) = PUP(from);
                    from = window - OFF;
                                PUP(out) = PUP(from);
                                PUP(out) = PUP(from);
                            from = window - OFF;
                                    PUP(out) = PUP(from);
                                PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                            PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                        PUP(out) = PUP(from);
                            PUP(out) = PUP(from);
    strm->next_in = in + OFF;
    strm->next_out = out + OFF;
Fixed code:     in = strm->next_in;
    out = strm->next_out;
            hold += (unsigned long)(*in++) << bits;
            hold += (unsigned long)(*in++) << bits;
            *out++ = (unsigned char)(here.val);
                    hold += (unsigned long)(*in++) << bits;
                hold += (unsigned long)(*in++) << bits;
                hold += (unsigned long)(*in++) << bits;
                    hold += (unsigned long)(*in++) << bits;
                        hold += (unsigned long)(*in++) << bits;
                                *out++ = 0;
                            *out++ = 0;
                                *out++ = *from++;
                    from = window;
                                *out++ = *from++;
                                *out++ = *from++;
                            from = window;
                                    *out++ = *from++;
                                *out++ = *from++;
                        *out++ = *from++;
                        *out++ = *from++;
                        *out++ = *from++;
                        *out++ = *from++;
                            *out++ = *from++;
                        *out++ = *from++;
                        *out++ = *from++;
                        *out++ = *from++;
                        *out++ = *from++;
                            *out++ = *from++;
    strm->next_in = in;
    strm->next_out = out;