Mask special bits from peer-supplied SCP receive mode

Author: yosuke-wolfssl

src/wolfscp.c

@@ -1005,8 +1005,9 @@ static int GetScpFileMode(WOLFSSH* ssh, byte* buf, word32 bufSz,
     }
 
     if (ret == WS_SUCCESS) {
-        /* store file mode */
-        ssh->scpFileMode = mode;
+        /* store file mode, masking off setuid/setgid/sticky bits from the
+         * peer-supplied value to match the send path */
+        ssh->scpFileMode = mode & WOLFSSH_MODE_MASK;
         /* eat trailing space */
         if (bufSz >= (word32)(idx +1))
             idx++;
@@ -1018,6 +1019,15 @@ static int GetScpFileMode(WOLFSSH* ssh, byte* buf, word32 bufSz,
 }
 
 
+#ifdef WOLFSSH_TEST_INTERNAL
+int wolfSSH_TestScpGetFileMode(WOLFSSH* ssh, byte* buf, word32 bufSz,
+        word32* inOutIdx)
+{
+    return GetScpFileMode(ssh, buf, bufSz, inOutIdx);
+}
+#endif /* WOLFSSH_TEST_INTERNAL */
+
+
 /* Locates first space present in given string (buf) and sets inOutIdx
  * to that offset.
  *

tests/unit.c

@@ -1222,6 +1222,81 @@ static int test_DoUserAuthBanner(void)
     return result;
 }
 
+#if defined(WOLFSSH_TEST_INTERNAL) && defined(WOLFSSH_SCP)
+/* Verify GetScpFileMode strips setuid/setgid/sticky bits from a peer-supplied
+ * SCP C/D record mode, matching the masking already done on the send path.
+ * The receive path cannot be exercised end-to-end because both peers mask the
+ * mode before transmitting, so this drives the parser directly. */
+static int test_ScpGetFileMode(void)
+{
+    WOLFSSH_CTX* ctx = NULL;
+    WOLFSSH* ssh = NULL;
+    static const char* hdrs[] = {
+        "C4755 0 f\n",  /* setuid set */
+        "D2755 0 d\n",  /* setgid set */
+        "D1755 0 d\n",  /* sticky set */
+        "D7777 0 d\n",  /* all special bits set */
+        "C0644 0 f\n"   /* ordinary mode, unaffected */
+    };
+    static const int expected[] = { 0755, 0755, 0755, 0777, 0644 };
+    /* records the parser must reject */
+    static const char* badHdrs[] = {
+        "C8755 0 f\n",  /* '8' is not an octal digit */
+        "X4755 0 f\n",  /* prefix is neither 'C' nor 'D' */
+        "C75"           /* shorter than the mode field */
+    };
+    int result = 0;
+    int ret;
+    int i;
+    word32 idx;
+
+    ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
+    if (ctx == NULL)
+        return -420;
+    ssh = wolfSSH_new(ctx);
+    if (ssh == NULL) {
+        wolfSSH_CTX_free(ctx);
+        return -421;
+    }
+
+    for (i = 0; i < (int)(sizeof(hdrs) / sizeof(hdrs[0])); i++) {
+        idx = 0;
+        ssh->scpFileMode = 0;
+        ret = wolfSSH_TestScpGetFileMode(ssh, (byte*)hdrs[i],
+                (word32)WSTRLEN(hdrs[i]), &idx);
+        if (ret != WS_SUCCESS) {
+            result = -422;
+            break;
+        }
+        if (ssh->scpFileMode != expected[i]) {
+            result = -423;
+            break;
+        }
+        /* index advances past the 'C'/'D', four mode octets, and the
+         * trailing space */
+        if (idx != 6) {
+            result = -424;
+            break;
+        }
+    }
+
+    for (i = 0; result == 0 &&
+            i < (int)(sizeof(badHdrs) / sizeof(badHdrs[0])); i++) {
+        idx = 0;
+        ret = wolfSSH_TestScpGetFileMode(ssh, (byte*)badHdrs[i],
+                (word32)WSTRLEN(badHdrs[i]), &idx);
+        if (ret != WS_BAD_ARGUMENT) {
+            result = -425;
+            break;
+        }
+    }
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+    return result;
+}
+#endif /* WOLFSSH_TEST_INTERNAL && WOLFSSH_SCP */
+
 static int test_ChannelPutData(void)
 {
     WOLFSSH_CTX* ctx = NULL;
@@ -4850,6 +4925,12 @@ int wolfSSH_UnitTest(int argc, char** argv)
     printf("ChannelPutData: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
     testResult = testResult || unitResult;
 
+#if defined(WOLFSSH_TEST_INTERNAL) && defined(WOLFSSH_SCP)
+    unitResult = test_ScpGetFileMode();
+    printf("ScpGetFileMode: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
+    testResult = testResult || unitResult;
+#endif
+
     unitResult = test_MsgHighwater();
     printf("MsgHighwater: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
     testResult = testResult || unitResult;

wolfssh/internal.h

@@ -1424,6 +1424,10 @@ enum WS_MessageIdLimits {
     WOLFSSH_API int wolfSSH_TestSendUserAuthFailure(WOLFSSH* ssh,
             byte partialSuccess);
     WOLFSSH_API int wolfSSH_TestHighwaterCheck(WOLFSSH* ssh, byte side);
+#ifdef WOLFSSH_SCP
+    WOLFSSH_API int wolfSSH_TestScpGetFileMode(WOLFSSH* ssh, byte* buf,
+            word32 bufSz, word32* inOutIdx);
+#endif /* WOLFSSH_SCP */
 #ifndef WOLFSSH_NO_DH
     WOLFSSH_API int wolfSSH_TestKeyAgreeDh_client(WOLFSSH* ssh, byte hashId,
             const byte* f, word32 fSz);