On Thu, Jan 2, 2014 at 11:13 PM, Ryan Mallon <[email protected]> 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 <[email protected]>
> ---
> 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 != ' '
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
