Am 28.03.23 um 17:12 schrieb Frank Lichtenheld:
Since we use strlen() to determine the length
and then check it ourselves, there is really
no point in using strncpy.
But the compiler might complain that we use
the output of strlen() for the length of
strncpy which is usually a sign for bugs:
error: ‘strncpy’ specified bound depends
on the length of the source argument
[-Werror=stringop-overflow=]
Warning was at least triggered for
mingw-gcc version 10-win32 20220113.
Signed-off-by: Frank Lichtenheld <fr...@lichtenheld.com>
---
src/openvpn/buffer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index d099795b..de40d690 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -317,9 +317,9 @@ buf_catrunc(struct buffer *buf, const char *str)
if (buf_forward_capacity(buf) <= 1)
{
int len = (int) strlen(str) + 1;
- if (len < buf_forward_capacity_total(buf))
+ if ((len > 0) && (len < buf_forward_capacity_total(buf)))
I hope you don't mind but there are a few things I would like to bring
to attention.
1. The first is clamping and wrapping around:
strlen(someptr) returns size_t, and this can be >= INT_MAX. Add +1 and
this wraps to INT_MIN.
While you now trap this with the new "len > 0", there does not seem to
be any error handling, in that the function silently skips its actual
function.
Is this what such a function should do? Or should it be "complain an
give up"?
{
- strncpynt((char *)(buf->data + buf->capacity - len), str, len);
+ memcpy((void *)(buf->data + buf->capacity - len), (void *)str,
(size_t)len);
2. This opens the door for another compiler warning, which is that you
are casting away the const-ness of "str". It is const char * when passed
in, but you cast it to void * (non-const). The 2nd argument of memcpy
would need to be cast to (const void *) to avoid that. memcpy is
declared (..., const void *restrict src, ...), so that's safe.
3. However, strncpynt would set buf->data[buf->capacity] = '\0', which
is now missing.
So: NAK, please revise on #3.
Cheers,
Matthias
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
