Zeroize SFTP file payload buffers before freeing

Author: yosuke-wolfssl

.github/workflows/os-check.yml

@@ -72,6 +72,7 @@ jobs:
           '',
           '--enable-all',
           '--enable-sftp',
+          '--enable-sftp --disable-sftp-zeroize',
           '--enable-scp',
           '--enable-keyboard-interactive',
           '--enable-shell',

configure.ac

@@ -151,6 +151,11 @@ AC_ARG_ENABLE([sftp],
     [AS_HELP_STRING([--enable-sftp],[Enable SFTP support (default: disabled)])],
     [ENABLED_SFTP=$enableval],[ENABLED_SFTP=no])
 
+# SFTP buffer zeroization
+AC_ARG_ENABLE([sftp-zeroize],
+    [AS_HELP_STRING([--disable-sftp-zeroize],[Disable zeroization of SFTP file payload buffers before free (default: enabled)])],
+    [ENABLED_SFTP_ZEROIZE=$enableval],[ENABLED_SFTP_ZEROIZE=yes])
+
 # SSHD
 AC_ARG_ENABLE([sshd],
     [AS_HELP_STRING([--enable-sshd],[Enable SSHD support (default: disabled)])],
@@ -235,6 +240,8 @@ AS_IF([test "x$ENABLED_SCP" = "xyes"],
       [AM_CPPFLAGS="$AM_CPPFLAGS -DWOLFSSH_SCP"])
 AS_IF([test "x$ENABLED_SFTP" = "xyes"],
       [AM_CPPFLAGS="$AM_CPPFLAGS -DWOLFSSH_SFTP"])
+AS_IF([test "x$ENABLED_SFTP_ZEROIZE" = "xno"],
+      [AM_CPPFLAGS="$AM_CPPFLAGS -DWOLFSSH_NO_SFTP_BUFFER_ZERO"])
 AS_IF([test "x$ENABLED_FWD" = "xyes"],
       [AM_CPPFLAGS="$AM_CPPFLAGS -DWOLFSSH_FWD"])
 AS_IF([test "x$ENABLED_TERM" = "xyes"],
@@ -342,6 +349,7 @@ AS_ECHO(["   * psuedo-terminal:           $ENABLED_TERM"])
 AS_ECHO(["   * echoserver shell support:  $ENABLED_SHELL"])
 AS_ECHO(["   * scp:                       $ENABLED_SCP"])
 AS_ECHO(["   * sftp:                      $ENABLED_SFTP"])
+AS_ECHO(["   * sftp buffer zeroize:       $ENABLED_SFTP_ZEROIZE"])
 AS_ECHO(["   * sshd:                      $ENABLED_SSHD"])
 AS_ECHO(["   * ssh client:                $ENABLED_SSHCLIENT"])
 AS_ECHO(["   * agent:                     $ENABLED_AGENT"])

src/wolfsftp.c

@@ -470,6 +470,12 @@ static int wolfSSH_SFTP_buffer_set_size(WS_SFTP_BUFFER* buffer, word32 sz)
         return WS_BAD_ARGUMENT;
     }
 
+#ifndef WOLFSSH_NO_SFTP_BUFFER_ZERO
+    /* wipe any payload in the region being trimmed off before shrinking */
+    if (buffer->data != NULL && sz < buffer->sz) {
+        ForceZero(buffer->data + sz, buffer->sz - sz);
+    }
+#endif
     buffer->sz = sz;
     return WS_SUCCESS;
 }
@@ -793,12 +799,15 @@ static int wolfSSH_SFTP_buffer_read(WOLFSSH* ssh, WS_SFTP_BUFFER* buffer,
 static void wolfSSH_SFTP_buffer_free(WOLFSSH* ssh, WS_SFTP_BUFFER* buffer)
 {
     if (ssh != NULL && buffer != NULL) {
-        buffer->idx = 0;
-        buffer->sz  = 0;
         if (buffer->data != NULL) {
+#ifndef WOLFSSH_NO_SFTP_BUFFER_ZERO
+            ForceZero(buffer->data, buffer->sz);
+#endif
             WFREE(buffer->data, ssh->ctx->heap, DYNTYPE_BUFFER);
             buffer->data = NULL;
         }
+        buffer->idx = 0;
+        buffer->sz  = 0;
     }
 }
 
@@ -1424,6 +1433,9 @@ static void wolfSSH_SFTP_RecvSetSend(WOLFSSH* ssh, byte* buf, int sz)
 
     /* free up existing data if needed */
     if (buf != state->buffer.data && state->buffer.data != NULL) {
+#ifndef WOLFSSH_NO_SFTP_BUFFER_ZERO
+        ForceZero(state->buffer.data, state->buffer.sz);
+#endif
         WFREE(state->buffer.data, ssh->ctx->heap, DYNTYPE_BUFFER);
         state->buffer.data = NULL;
     }