[Openvpn-devel,v2,4/5] tapctl: Resolve MSVC C4996 warnings

Message ID 20210322074359.527-1-simon@rozman.si
State Accepted
Headers show
Series
  • Untitled series #1072
Related show

Commit Message

tincantech via Openvpn-devel March 22, 2021, 7:43 a.m.
wcsncat() was declared unsafe in favour of wcsncat_s(). However, the
string concatenation follows the string length check, making wcsncat()
safe too. Code analysis is just not smart enough (yet) to detect this.

The code was refactored to use wcscat_s() MSVC is considering as "safe".

Signed-off-by: Simon Rozman <simon@rozman.si>
---
 src/tapctl/tap.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Gert Doering March 22, 2021, 9:10 a.m. | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

It's the same code change as in v1, just the comment rewritten as Arne 
asked for (I found the other one okay-ish, but this is more in line with 
our other code, and indeed "less thinking required on read") and a new
helper variable for the total length.

Code looks good, test build on Ubuntu 18/MingW also had no complaints.

Your patch has been applied to the master branch.

commit ef5b922aedead7159718ddbf40fc9f5be1b2b203
Author: Simon Rozman via Openvpn-devel
Date:   Mon Mar 22 08:43:59 2021 +0100

     tapctl: Resolve MSVC C4996 warnings

     Signed-off-by: Simon Rozman <simon@rozman.si>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20210322074359.527-1-simon@rozman.si>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21774.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering March 22, 2021, 9:17 a.m. | #2
Hi,

On Mon, Mar 22, 2021 at 10:10:42AM +0100, Gert Doering wrote:
> Acked-by: Gert Doering <gert@greenie.muc.de>
> 
> It's the same code change as in v1, just the comment rewritten as Arne 
> asked for (I found the other one okay-ish, but this is more in line with 
> our other code, and indeed "less thinking required on read") and a new
> helper variable for the total length.
> 
> Code looks good, test build on Ubuntu 18/MingW also had no complaints.
> 
> Your patch has been applied to the master branch.
> 
> commit ef5b922aedead7159718ddbf40fc9f5be1b2b203
> Author: Simon Rozman via Openvpn-devel
> Date:   Mon Mar 22 08:43:59 2021 +0100

My local tree was in a slightly funny state (the tip commit had a new
ID due to an accidential "git commit --amend" on the wrong head), so
I had to rebase and this commit got a new ID

commit e5e9a07e8baee4065b7dfd65736bfa77b8329cfc (HEAD -> master, stable/master, mattock/master, gitlab/master, github/master)
Author: Simon Rozman via Openvpn-devel <openvpn-devel@lists.sourceforge.net>
Date:   Mon Mar 22 08:43:59 2021 +0100

    tapctl: Resolve MSVC C4996 warnings


(and I forgot to change the From: line again, sorry, again)


Must be Monday :-(

gert

Patch

diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
index dd4a10a3..563c07f6 100644
--- a/src/tapctl/tap.c
+++ b/src/tapctl/tap.c
@@ -2,7 +2,7 @@ 
  *  tapctl -- Utility to manipulate TUN/TAP adapters on Windows
  *            https://community.openvpn.net/openvpn/wiki/Tapctl
  *
- *  Copyright (C) 2018-2020 Simon Rozman <simon@rozman.si>
+ *  Copyright (C) 2018-2021 Simon Rozman <simon@rozman.si>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2
@@ -73,14 +73,15 @@  find_function(const WCHAR *libname, const char *funcname, HMODULE *m)
        return NULL;
     }
 
-    size_t len = _countof(libpath) - wcslen(libpath) - 1;
-    if (len < wcslen(libname) + 1)
+    /* +1 for the path seperator '\' */
+    const size_t path_length = wcslen(libpath) + 1 + wcslen(libname);
+    if (path_length >= _countof(libpath))
     {
        SetLastError(ERROR_INSUFFICIENT_BUFFER);
        return NULL;
     }
-    wcsncat(libpath, L"\\", len);
-    wcsncat(libpath, libname, len-1);
+    wcscat_s(libpath, _countof(libpath), L"\\");
+    wcscat_s(libpath, _countof(libpath), libname);
 
     *m = LoadLibraryW(libpath);
     if (*m == NULL)