• Show log

    Commit

  • Hash : 33e6c402
    Author : Patrick Steinhardt
    Date : 2019-11-28T15:26:36

    patch_parse: fix out-of-bounds reads caused by integer underflow
    
    The patch format for binary files is a simple Base85 encoding with a
    length byte as prefix that encodes the current line's length. For each
    line, we thus check whether the line's actual length matches its
    expected length in order to not faultily apply a truncated patch. This
    also acts as a check to verify that we're not reading outside of the
    line's string:
    
    	if (encoded_len > ctx->parse_ctx.line_len - 1) {
    		error = git_parse_err(...);
    		goto done;
    	}
    
    There is the possibility for an integer underflow, though. Given a line
    with a single prefix byte, only, `line_len` will be zero when reaching
    this check. As a result, subtracting one from that will result in an
    integer underflow, causing us to assume that there's a wealth of bytes
    available later on. Naturally, this may result in an out-of-bounds read.
    
    Fix the issue by checking both `encoded_len` and `line_len` for a
    non-zero value. The binary format doesn't make use of zero-length lines
    anyway, so we need to know that there are both encoded bytes and
    remaining characters available at all.
    
    This patch also adds a test that works based on the last error message.
    Checking error messages is usually too tightly coupled, but in fact
    parsing the patch failed even before the change. Thus the only
    possibility is to use e.g. Valgrind, but that'd result in us not
    catching issues when run without Valgrind. As a result, using the error
    message is considered a viable tradeoff as we know that we didn't start
    decoding Base85 in the first place.