Ensure a key is set for ARC4 operations

Fixes F-5378

doc/dox_comments/header_files/arc4.h

@@ -7,7 +7,10 @@
     Before this method may be called, one must first initialize the ARC4
     structure using wc_Arc4SetKey.
 
-    \return none
+    \return 0 Returned upon successfully processing the message.
+    \return BAD_FUNC_ARG If arc4, out, or in is NULL.
+    \return MISSING_KEY If no key has been set on the ARC4 structure with
+    wc_Arc4SetKey.
 
     \param arc4 pointer to the ARC4 structure used to process the message
     \param out pointer to the output buffer in which to store the

tests/api/test_arc4.c

@@ -104,6 +104,43 @@ int test_wc_Arc4Process(void)
 
 } /* END test_wc_Arc4Process */
 
+/*
+ * Regression test for issue 5378: wc_Arc4Process must refuse to run unless a
+ * key has been configured with wc_Arc4SetKey(), both before SetKey and after
+ * Free. Otherwise the keystream is all-zero and ARC4 silently copies the
+ * plaintext into the ciphertext.
+ */
+int test_wc_Arc4Process_no_key(void)
+{
+    EXPECT_DECLS;
+#ifndef NO_RC4
+    Arc4 arc4;
+    const char* key = "\x01\x23\x45\x67\x89\xab\xcd\xef";
+    int keyLen = 8;
+    const char* input = "\x01\x23\x45\x67\x89\xab\xcd\xef";
+    byte cipher[8];
+
+    XMEMSET(&arc4, 0, sizeof(arc4));
+    XMEMSET(cipher, 0, sizeof(cipher));
+
+    /* Processing without a key after init must fail, not silently copy. */
+    ExpectIntEQ(wc_Arc4Init(&arc4, NULL, INVALID_DEVID), 0);
+    ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen),
+        WC_NO_ERR_TRACE(MISSING_KEY));
+
+    /* After a key is set, processing succeeds. */
+    ExpectIntEQ(wc_Arc4SetKey(&arc4, (byte*)key, (word32)keyLen), 0);
+    ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen), 0);
+
+    /* After free, the keyed state is cleared and processing must fail again. */
+    wc_Arc4Free(&arc4);
+    ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen),
+        WC_NO_ERR_TRACE(MISSING_KEY));
+#endif
+    return EXPECT_RESULT();
+
+} /* END test_wc_Arc4Process_no_key */
+
 
 #include <wolfssl/wolfcrypt/random.h>
 

tests/api/test_arc4.h

@@ -26,11 +26,13 @@
 
 int test_wc_Arc4SetKey(void);
 int test_wc_Arc4Process(void);
+int test_wc_Arc4Process_no_key(void);
 int test_wc_Arc4_MonteCarlo(void);
 
 #define TEST_ARC4_DECLS                                     \
     TEST_DECL_GROUP("arc4", test_wc_Arc4SetKey),            \
     TEST_DECL_GROUP("arc4", test_wc_Arc4Process),           \
+    TEST_DECL_GROUP("arc4", test_wc_Arc4Process_no_key),    \
     TEST_DECL_GROUP("arc4", test_wc_Arc4_MonteCarlo)
 
 #endif /* WOLFCRYPT_TEST_ARC4_H */

tests/api/test_ossl_cipher.c

@@ -965,7 +965,9 @@ int test_wolfSSL_RC4(void)
     wolfSSL_RC4(NULL, 0, data, enc);
 
     ExpectIntEQ(1, 1);
-    for (i = 0; EXPECT_SUCCESS() && (i <= sizeof(key)); i++) {
+    /* Start at a key length of 1: a zero-length key is invalid and leaves the
+     * Arc4 object without a key set, so encrypt/decrypt is a no-op. */
+    for (i = 1; EXPECT_SUCCESS() && (i <= sizeof(key)); i++) {
         for (j = 0; EXPECT_SUCCESS() && (j <= sizeof(data)); j++) {
             XMEMSET(enc, 0, sizeof(enc));
             XMEMSET(dec, 0, sizeof(dec));

wolfcrypt/src/arc4.c

@@ -67,6 +67,8 @@ int wc_Arc4SetKey(Arc4* arc4, const byte* key, word32 length)
             keyIndex = 0;
     }
 
+    arc4->keySet = 1;
+
     return ret;
 }
 
@@ -102,6 +104,10 @@ int wc_Arc4Process(Arc4* arc4, byte* out, const byte* in, word32 length)
     }
 #endif
 
+    if (!arc4->keySet) {
+        return MISSING_KEY;
+    }
+
     x = arc4->x;
     y = arc4->y;
 
@@ -123,6 +129,7 @@ int wc_Arc4Init(Arc4* arc4, void* heap, int devId)
         return BAD_FUNC_ARG;
 
     arc4->heap = heap;
+    arc4->keySet = 0;
 
 #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ARC4)
     ret = wolfAsync_DevCtxInit(&arc4->asyncDev, WOLFSSL_ASYNC_MARKER_ARC4,
@@ -148,6 +155,7 @@ void wc_Arc4Free(Arc4* arc4)
     ForceZero(arc4->state, sizeof(arc4->state));
     arc4->x = 0;
     arc4->y = 0;
+    arc4->keySet = 0;
 }
 
 #endif /* NO_RC4 */

wolfssl/openssl/rc4.h

@@ -40,9 +40,9 @@
 typedef struct WOLFSSL_RC4_KEY {
     /* big enough for Arc4 from wolfssl/wolfcrypt/arc4.h */
 #ifdef WC_NO_PTR_INT_CAST
-    void* holder[(288 + WC_ASYNC_DEV_SIZE) / sizeof(void*)];
+    void* holder[(296 + WC_ASYNC_DEV_SIZE) / sizeof(void*)];
 #else
-    void* holder[(272 + WC_ASYNC_DEV_SIZE) / sizeof(void*)];
+    void* holder[(280 + WC_ASYNC_DEV_SIZE) / sizeof(void*)];
 #endif
 } WOLFSSL_RC4_KEY;
 

wolfssl/wolfcrypt/arc4.h

@@ -51,6 +51,7 @@ typedef struct Arc4 {
     WC_ASYNC_DEV asyncDev;
 #endif
     void* heap;
+    WC_BITFIELD keySet:1;    /* set to 1 once a key has been configured */
 } Arc4;
 
 WOLFSSL_API int wc_Arc4Process(Arc4* arc4, byte* out, const byte* in,