scp and sftp build errors with gcc 8.3.0

[bagder]

I build wolfssh straight from git, currently on commit 57a869e (dated April 22) on Debian Linux 64 bit (unstable) with gcc 8.3.0 and then I get this set of build errors:

$ CPPFLAGS=-I/home/daniel/build-wolfssl/include LDFLAGS=-L/home/daniel/build-wolfssl/lib ./configure --prefix=/home/daniel/build-wolfssh --enable-scp --enable-sftp --disable-examples
[cut]
$ make
make -j9  all-am
make[1]: Entering directory '/home/daniel/src/bagder-wolfssh'
  CC       src/libwolfssh_la-wolfsftp.lo
  CC       src/libwolfssh_la-wolfscp.lo
In file included from ./wolfssh/ssh.h:37,
                 from ./wolfssh/wolfscp.h:27,
                 from src/wolfscp.c:34:
src/wolfscp.c: In function ‘ParseBasePathHelper’:
./wolfssh/port.h:228:35: error: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
         #define WSTRNCPY(s1,s2,n) strncpy((s1),(s2),(n))
                                   ^~~~~~~~~~~~~~~~~~~~~~
src/wolfscp.c:1074:9: note: in expansion of macro ‘WSTRNCPY’
         WSTRNCPY(buf, ssh->scpBasePath, sz);
         ^~~~~~~~
./wolfssh/port.h:207:31: note: length computed here
     #define WSTRLEN(s1)       strlen((s1))
                               ^~~~~~~~~~~~
src/wolfscp.c:1067:24: note: in expansion of macro ‘WSTRLEN’
         int  sz = (int)WSTRLEN(ssh->scpBasePath);
                        ^~~~~~~
src/wolfscp.c: In function ‘wolfSSH_SetScpErrorMsg’:
./wolfssh/port.h:228:35: error: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
         #define WSTRNCPY(s1,s2,n) strncpy((s1),(s2),(n))
                                   ^~~~~~~~~~~~~~~~~~~~~~
src/wolfscp.c:710:9: note: in expansion of macro ‘WSTRNCPY’
         WSTRNCPY(value + 1, message, valueSz);
         ^~~~~~~~
./wolfssh/port.h:207:31: note: length computed here
     #define WSTRLEN(s1)       strlen((s1))
                               ^~~~~~~~~~~~
src/wolfscp.c:700:27: note: in expansion of macro ‘WSTRLEN’
         valueSz = (word32)WSTRLEN(message);
                           ^~~~~~~
In file included from ./wolfssh/ssh.h:34,
                 from ./wolfssh/wolfsftp.h:29,
                 from src/wolfsftp.c:26:
src/wolfsftp.c: In function ‘wolfSSH_SFTP_RecvRealPath’:
/home/daniel/build-wolfssl/include/wolfssl/wolfcrypt/types.h:372:35: error: ‘strncpy’ specified bound 256 equals destination size [-Werror=stringop-truncation]
         #define XSTRNCPY(s1,s2,n) strncpy((s1),(s2),(n))
                                   ^~~~~~~~~~~~~~~~~~~~~~
src/wolfsftp.c:922:13: note: in expansion of macro ‘XSTRNCPY’
             XSTRNCPY(wd, ssh->sftpDefaultPath, sizeof(wd));
             ^~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:1263: src/libwolfssh_la-wolfscp.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
cc1: all warnings being treated as errors
make[1]: *** [Makefile:1270: src/libwolfssh_la-wolfsftp.lo] Error 1
make[1]: Leaving directory '/home/daniel/src/bagder-wolfssh'
make: *** [Makefile:851: all] Error 2

I've looked at the code around these errors and I think the code is fine and already does the necessary boundary checks. I managed to hush up gcc with the following patch, but I'm not at all convinced that it is good enough so I leave it here just to show what I did and not because I think it necessarily is the right fix:

diff --git a/src/wolfscp.c b/src/wolfscp.c
index 186d966..238163f 100644
--- a/src/wolfscp.c
+++ b/src/wolfscp.c
@@ -705,11 +705,11 @@ WOLFSSH_API int wolfSSH_SetScpErrorMsg(WOLFSSH* ssh, const char* message)
             ret = WS_MEMORY_E;
     }
 
     if (ret == WS_SUCCESS) {
         /* leave room for cmd at beginning, add \n\0 at end */
-        WSTRNCPY(value + 1, message, valueSz);
+        WSTRNCPY(value + 1, message, valueSz - 1);
         *(value + valueSz + 1) = '\n';
         *(value + valueSz + 2) = '\0';
 
         if (ssh->scpConfirmMsg != NULL) {
             WFREE(ssh->scpConfirmMsg, ssh->ctx->heap, DYNTYPE_STRING);
@@ -1069,11 +1069,11 @@ static int ParseBasePathHelper(WOLFSSH* ssh, int cmdSz)
 
         if (sz > (int)sizeof(buf)) {
             return WS_BUFFER_E;
         }
 
-        WSTRNCPY(buf, ssh->scpBasePath, sz);
+        WSTRNCPY(buf, ssh->scpBasePath, sizeof(buf)-1);
         buf[sz] = '\0';
         WSTRNCAT(buf, "/..", sizeof("/.."));
         clean_path(buf);
 
         idx = (int)WSTRLEN(buf) + 1; /* +1 for delimiter */
diff --git a/src/wolfsftp.c b/src/wolfsftp.c
index f3ee822..86979fa 100644
--- a/src/wolfsftp.c
+++ b/src/wolfsftp.c
@@ -917,11 +917,11 @@ static int wolfSSH_SFTP_RecvRealPath(WOLFSSH* ssh, int reqId, byte* data,
 
     /* get working directory in the case of receiving non absolute path */
     if (r[0] != '/' && r[1] != ':') {
         char wd[WOLFSSH_MAX_FILENAME];
         if (ssh->sftpDefaultPath) {
-            XSTRNCPY(wd, ssh->sftpDefaultPath, sizeof(wd));
+            XSTRNCPY(wd, ssh->sftpDefaultPath, sizeof(wd)-1);
         }
         else {
         #ifndef USE_WINDOWS_API
             if (WGETCWD(wd, WOLFSSH_MAX_FILENAME) == NULL) {
                 WLOG(WS_LOG_SFTP, "Unable to get current working directory");

[JacobBarthelmeh]

Thanks for the report. A fix has been merged in.