chicken-hackers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Chicken-hackers] [PATCH] Prevent uninitialized memory access when m


From: Peter Bex
Subject: Re: [Chicken-hackers] [PATCH] Prevent uninitialized memory access when make-ing a srfi-4 vector
Date: Sun, 12 Mar 2017 16:42:00 +0100
User-agent: Mutt/1.5.23 (2014-03-12)

On Fri, Mar 10, 2017 at 04:36:25PM +0100, lemonboy wrote:
> Hello hackers,
> during one hell of a trip trough the srfi-4 code I noticed that a
> check was missing when we try to allocate a srfi-4 vector on the heap.
> You can see by yourself that `(make-u8vector -7 #f #t)' (on a 64-bit
> platform) ends up malloc-ing a 1-byte buffer and then calls
> C_block_header_init on it, writing over a chunk of memory that's over
> the buffer end.  Attached is a patch against the chicken-5 branch, I
> think it should apply cleanly to the 4.x branch too.

Thanks for the patch.  Attached is a different patch that fixes this
issue, because I think the error message I got after applying your patch
is confusing/misleading:

#;2> (make-u8vector -1 1 #t)
Error: (make-u8vector) not enough memory - cannot allocate external number 
vector: -1

The attached patch simplifies the type and range checking by moving it
to the "alloc" procedure, to reduce code duplication.  The error message
is taken from make-vector, and it simply says "size is negative".
I decided to keep the header size mask check in C, with the out of memory
error message, which is slightly wrong but less invasive than properly
fixing it.  I think we need to take a good look at how barf() works, and
see if we can get rid of it or simplify it, and make it externally
callable.  But this is something for another patch :)

Because this should go to master as well, and we've only started
"officially" requiring C99 for CHICKEN 5 I've moved the declaration
of the buffer up to the start of the function in that branch.
This is probably overly conservative, but better safe than sorry.

I also took the opportunity to get rid of some square brackets in
this code :)

Considering this is a security fix I also added an entry to NEWS.

Cheers,
Peter

Attachment: 0001-Add-bound-checking-to-all-srfi-4-vector-allocations.chicken-5.patch
Description: Text Data

Attachment: 0001-Add-bound-checking-to-all-srfi-4-vector-allocations.master.patch
Description: Text Data

Attachment: signature.asc
Description: Digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]