Hi, On 29/12/17 17:26, Steffan Karger wrote: > Hi, > > On 28-12-17 23:51, Antonio Quartulli wrote: >> On 01/07/17 20:54, Steffan Karger wrote: >>> As pointed out in finding OVPN-05 of the cryptograpy engineering >>> audit (funded by Private Internet Access), >>> buffer_list_aggregate_separator() could perform a 0-byte malloc >>> when called with a list of 0-length buffers and a "" separator. >>> If other could would later try to access that buffer memory, this >>> would result in undefined behaviour. To prevent this, always >>> malloc() 1 byte. >>> >>> To simplify as we go, use alloc_buf() to allocate the buffer. >>> This has the additional benefit that the actual buffer data (not >>> the contents) is zero-terminated, because alloc_buf() calls >>> calloc() and we have 1 extra byte of data. >>> >>> Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com> > > >> Does it really make sense to continue the execution if we receive a >> list of empty buffers? Shouldn't we report this as an error? > > Data is pushed to buffer lists from e.g. tls_send_payload(), where I'm > not sure that 'sending' 0-byte payload is an invalid operation. So > instead of trying to figure out if that's possible, I prefer to just > make this function deal with it.
Yeah, makes sense: better let the external layer deal with the logic. > > (Consider e.g. 0/n record splitting, which might want to 'send' > 0-length data.) > > I'll send follow-up patches to take care of your other remarks. > Thanks! Cheers, -- Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
