Skip to content

Commit

Permalink
buffer: use memcpy in buf_catrunc
Browse files Browse the repository at this point in the history
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.

Also change the type of len to size_t
which avoids potential problems with
signed overflow.

v2:
 - make len size_t and change code to avoid any theoretical overflows
 - remove useless casts
v3:
 - fix off-by-one introduced by v2 %)
v4:
 - ignore unsigned overflow to simplify code

Change-Id: If4a67adac4d2e870fd719b58075d39efcd67c671
Signed-off-by: Frank Lichtenheld <[email protected]>
Acked-by: Antonio Quartulli <[email protected]>
Acked-by: Heiko Hund <[email protected]>
Acked-by: Gert Doering <[email protected]>
(cherry picked from commit c89a97e)
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/[email protected]/msg27085.html
Signed-off-by: Gert Doering <[email protected]>
  • Loading branch information
flichtenheld authored and cron2 committed Sep 23, 2023
1 parent 3660564 commit 9462191
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/openvpn/buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,10 @@ buf_catrunc(struct buffer *buf, const char *str)
{
if (buf_forward_capacity(buf) <= 1)
{
int len = (int) strlen(str) + 1;
size_t len = strlen(str) + 1;
if (len < buf_forward_capacity_total(buf))
{
strncpynt((char *)(buf->data + buf->capacity - len), str, len);
memcpy(buf->data + buf->capacity - len, str, len);
}
}
}
Expand Down

0 comments on commit 9462191

Please sign in to comment.