| Message ID | 20210322074359.527-1-simon@rozman.si |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
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
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
Hi, On Mon, Mar 22, 2021 at 10:17:17AM +0100, Gert Doering wrote: > 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) By popular demand (Lev) this was now cherry-picked to 2.5 as well. commit 64547d552dbcadd826a30a4ba122d590d87504f6 (HEAD -> release/2.5) Author: Simon Rozman <simon@rozman.si> Date: Mon Mar 22 08:43:59 2021 +0100 tapctl: Resolve MSVC C4996 warnings gert
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)
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(-)