-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 18/09/16 17:19, Ilya Shipitsin wrote:
> --- src/openvpn/buffer.c | 6 ++++-- 1 file changed, 4
> insertions(+), 2 deletions(-)
>
> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index
> 52c6ab9..57bded9 100644 --- a/src/openvpn/buffer.c +++
> b/src/openvpn/buffer.c @@ -438,10 +438,12 @@ format_hex_ex (const
> uint8_t *data, int size, int maxoutput, unsigned int
> space_break_flags, const char* separator, struct gc_arena *gc) { -
> struct buffer out = alloc_buf_gc (maxoutput ? maxoutput : + int i;
> + struct buffer out; + ASSERT( maxoutput > 0 || separator != NULL
> ); + out = alloc_buf_gc (maxoutput ? maxoutput : ((size * 2) +
> (size / (space_break_flags & FHE_SPACE_BREAK_MASK)) * (int) strlen
> (separator) + 2), gc); - int i; for (i = 0; i < size; ++i) { if
> (separator && i && !(i % (space_break_flags &
> FHE_SPACE_BREAK_MASK)))
>
This looks dangerous and somewhat wrong. This can actually stop a
server completely if the ASSERT() check fails. I'm not sure we want
to do that.
The ASSERT() also expects maxoutput to be higher than 0, while the
alloc_buf_gc() supports/allows maxoutput to be 0, which again confirms
my bad feeling about this. If maxoutput == 0, alloc_buf_gc() will
actually allocate a reasonable buffer regardless.
I do see that there might be an issue if separator == NULL
(strlen(NULL) will segfault), so this needs to be checked far more
carefully and compared against all the callers of format_hex_ex().
The question which pops up in my head is: Is this patch purely
targeted to silence a code analyser warning? If so, this is most
likely not the right fix for the OpenVPN code base.
- --
kind regards,
David Sommerseth
OpenVPN Technologies, Inc
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBAgAGBQJX3q9BAAoJEIbPlEyWcf3yGkwQAKKZbI2idz6a7h5ZkbQJCHMk
JDNIpdR6XTFmG16jBrycYeew+9IRvfwpF4vh2/QPaRjDNLnKvPHP+9uEetqYmUQi
rWzJyg3nzXSEUhFRSbdJcOK7zEWoiUUsSTiBKIYP6D5qD+QBQOZaQseCCzafrt5b
IKrUesWMWPXAjQBYbUvcc801+JR5S/gSaSbk9TuO8Nemiry/GW/pSt0OVTggPdcm
LiUyutQ8HXeXS9uAL32FLqFH3qq7QTHjxdVoKGGx9w/lmznqp+EaKCuANsWnt/PW
ZTzVYM5YG5jI8XNM/wyCklu5n3yqJ37Noxd3ZRSD6ATTL/MExVgWINOOGA7f+Pj3
P6g4ZSc//zaWUCRpUKg4O8h6jaO+0stDS5eKDpyUCHYRHRdMI4IGLuwzIzKmxWq9
LpNGGY3Ikpf5IM+i2R04HctnNp+izEeW3vBCKnBBUHSyegRQGQ191Z5QC3SPk6VP
HwxkHCZiOsie8WsP0Sl2dur1GjV9uyslVTkmt1GBVuoOGxpmZLJRJRayO70goZ17
ToavpadDjt193g79j1/dpE7OCPcCitso/+egxVzDWPTtLj+7rS/gfdJBNdCHr7xK
Pf8KKbpD7yEbEln0K7wzsjVG749qZe103KmrhZulvu3H5+SrbT5ALhroaYidhRpg
lLcx4g4A+nZjM6kabBGy
=QvxV
-----END PGP SIGNATURE-----
------------------------------------------------------------------------------
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
