[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
0001-Add-bound-checking-to-all-srfi-4-vector-allocations.chicken-5.patch
Description: Text Data
0001-Add-bound-checking-to-all-srfi-4-vector-allocations.master.patch
Description: Text Data
signature.asc
Description: Digital signature