Fix heap-buffer-overflow in pglz_decompress() on corrupt input.

When decoding a match tag, pglz_decompress() reads 2 bytes (or 3
for extended-length matches) from the source buffer before checking
whether enough data remains.  The existing bounds check (sp > srcend)
occurs after the reads, so truncated compressed data that ends
mid-tag causes a read past the allocated buffer.

Fix by validating that sufficient source bytes are available before
reading each part of the match tag.  The post-read sp > srcend
check is no longer needed and is removed.

Found by fuzz testing with libFuzzer and AddressSanitizer.

diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index 75d529df5e3b8e11b56d5743fd009d4bff3cdd4c..c8e9ef9aa3b308783e749ada5836b869f487d55b 100644 (file)
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -727,22 +727,33 @@ pglz_decompress(const char *source, int32 slen, char *dest,
                                int32           len;
                                int32           off;
 
+                               /*
+                                * A match tag is at least 2 bytes; if the length nibble is
+                                * 0x0f the tag is 3 bytes (extended length).  Verify we have
+                                * enough source data before reading them.
+                                */
+                               if (unlikely(sp + 2 > srcend))
+                                       return -1;
+
                                len = (sp[0] & 0x0f) + 3;
                                off = ((sp[0] & 0xf0) << 4) | sp[1];
                                sp += 2;
                                if (len == 18)
+                               {
+                                       if (unlikely(sp >= srcend))
+                                               return -1;
                                        len += *sp++;
+                               }
 
                                /*
-                                * Check for corrupt data: if we fell off the end of the
-                                * source, or if we obtained off = 0, or if off is more than
-                                * the distance back to the buffer start, we have problems.
-                                * (We must check for off = 0, else we risk an infinite loop
-                                * below in the face of corrupt data.  Likewise, the upper
-                                * limit on off prevents accessing outside the buffer
-                                * boundaries.)
+                                * Check for corrupt data: if we obtained off = 0, or if off
+                                * is more than the distance back to the buffer start, we have
+                                * problems.  (We must check for off = 0, else we risk an
+                                * infinite loop below in the face of corrupt data. Likewise,
+                                * the upper limit on off prevents accessing outside the
+                                * buffer boundaries.)
                                 */
-                               if (unlikely(sp > srcend || off == 0 ||
+                               if (unlikely(off == 0 ||
                                                         off > (dp - (unsigned char *) dest)))
                                        return -1;