[PATCH 3/4] tar: Fix incorrect size check

[Ryan Mallon]

On 11/01/14 06:02, Denys Vlasenko wrote:

> On Thu, Jan 2, 2014 at 11:13 PM, Ryan Mallon <rmallon at gmail.com> wrote:
>> The function process_pax_header() in get_header_tar.c incorrectly
>> sanity checks the length of the record with:
>>
>>   (int)sz < 0
>>
>> If the value of len is 0xffffffff - n, then sz will increase by n + 1
>> bypassing the check. The pointer p will also decrease by n + 1
>> allowing a series of NUL byte writes to arbitrary locations below the
>> allocated buffer.
>>
>> Fix this by instead checking that len does not exceed size. Also do
>> the sanity checks before modifying sz or p.
>>
>> Signed-off-by: Ryan Mallon <rmallon at gmail.com>
>> ---
>>  archival/libarchive/get_header_tar.c |    8 +++++---
>>  1 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/archival/libarchive/get_header_tar.c b/archival/libarchive/get_header_tar.c
>> index bc09756..5956d01 100644
>> --- a/archival/libarchive/get_header_tar.c
>> +++ b/archival/libarchive/get_header_tar.c
>> @@ -113,9 +113,7 @@ static void process_pax_hdr(archive_handle_t *archive_handle, unsigned sz, int g
>>                 /* expect errno to be EINVAL, because the character
>>                  * following the digits should be a space
>>                  */
>> -               p += len;
>> -               sz -= len;
>> -               if ((int)sz < 0
>> +               if (len > sz
>>                  || len == 0
>>                  || errno != EINVAL
>>                  || *end != ' '
>> @@ -126,6 +124,10 @@ static void process_pax_hdr(archive_handle_t *archive_handle, unsigned sz, int g
>>                         //              archive_handle->offset - (sz + len));
>>                         break;
>>                 }
>> +
>> +               p += len;
>> +               sz -= len;
>> +
> 
> 
> +22 bytes on x86 :/
> 
> How about this?
> 
>                 p += len;
>                 sz -= len;
> -               if ((int)sz < 0
> +               if (
> +               /** (int)sz < 0 - not good enough for huge malicious
> VALUE of 2^32-1 */
> +                   (int)(sz|len) < 0 /* this works */
>                  || len == 0
>                  || errno != EINVAL
>                  || *end != ' '


That doesn't work. Try:

  sz = 512
  len = 0x7fffffff

Will result in sz being set to 0x7ffffdff and passing the check.

~Ryan