Project

General

Profile

Bug #4936

lz4 could theoretically overflow a pointer with a certain input

Added by Dan McDonald over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Immediate
Assignee:
Category:
-
Start date:
2014-06-24
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

From a bug reporter:

An integer overflow can occur when processing any variant of a "literal run" 
in the affected function. 

Exploitation can result when a "literal run" generates an integer that
would be interpreted as a signed long when applied to a pointer. The effect
of this is a pointer that accesses memory ahead of the start of the output
buffer. 

                /* get runlength */
                token = *ip++;
                if ((length = (token >> ML_BITS)) == RUN_MASK) {
                        int s = 255;
                        while ((ip < iend) && (s == 255)) {
                                s = *ip++;
                                length += s;
                        }
                }

Approximately sixteen megabytes worth of 0xFF bytes will cause the 
'length' variable to set its signed bit. Even though this variable is
unsigned, it will cause the pointer 'cpy' to overflow during the operation
in the following code:
                /* copy literals */
                cpy = op + length;

The error condition that follows is passed because of the unexpected values,
triggering a call to LZ4_WILDCOPY. Because of a bad check in this macro, only
four bytes will be copied. 

This bypasses a crash or an infinite copy loop and allows an attacker to over-
write only four bytes of memory before exiting the loop. 

The filer assumes 32-bit pointers, but regardless, there is a safety issue here that with ZFS-scale large files, could affect the LZ4 code.

History

#1

Updated by Matthew Ahrens over 5 years ago

Given that we only LZ4 compress blocks, which can be at most 128KB, this sounds like a theoretical problem (which nonetheless should be fixed).

Therefore I think that use of scary words like "exploitation" and "attacker" are inappropriate here, as there is no actual exploit or attack that can exercise this code.

That said, I don't quite understand the problem... it looks like 'length' is the run-length in bytes. Therefore, how could it be > 2^32 (4GB) when the uncompressed size is only 16 MB? Are you saying that there could be a malformed block which says that it decompresses into >2^32 bytes of data? How would such a malformed block be generated? Presumably not by any ZFS code that is even remotely correct. A non-privileged user can not write to the raw disk to construct such an incorrect block (and modify the checksums in all the indirect blocks that point to it, including the uberblock).

#2

Updated by Electric Monk over 5 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 58d0718061c87e3d647c891ec5281b93c08dba4e

commit  58d0718061c87e3d647c891ec5281b93c08dba4e
Author: Dan McDonald <danmcd@omniti.com>
Date:   2014-06-26T14:46:47.000Z

    4936 lz4 could theoretically overflow a pointer with a certain input
    Reviewed by: Saso Kiselkov <skiselkov.ml@gmail.com>
    Reviewed by: Keith Wesolowski <keith.wesolowski@joyent.com>
    Approved by: Gordon Ross <gordon.ross@nexenta.com>

Also available in: Atom PDF