Merge pull request #1340 from mpg/fix-string-to-names-uaf-3.6
[3.6] Fix string to names memory management
diff --git a/ChangeLog.d/pkcs7-padding-side-channel-fix.txt b/ChangeLog.d/pkcs7-padding-side-channel-fix.txt
new file mode 100644
index 0000000..b813b84
--- /dev/null
+++ b/ChangeLog.d/pkcs7-padding-side-channel-fix.txt
@@ -0,0 +1,4 @@
+Security
+ * Fix a timing side channel in the implementation of PKCS#7 padding
+ which would allow an attacker who can request decryption of arbitrary
+ ciphertexts to recover the plaintext through a timing oracle attack.
diff --git a/library/cipher.c b/library/cipher.c
index 7f4c121..2ae01dd 100644
--- a/library/cipher.c
+++ b/library/cipher.c
@@ -14,6 +14,7 @@
#if defined(MBEDTLS_CIPHER_C)
#include "mbedtls/cipher.h"
+#include "cipher_invasive.h"
#include "cipher_wrap.h"
#include "mbedtls/platform_util.h"
#include "mbedtls/error.h"
@@ -838,8 +839,14 @@
}
}
-static int get_pkcs_padding(unsigned char *input, size_t input_len,
- size_t *data_len)
+/*
+ * Get the length of the PKCS7 padding.
+ *
+ * Note: input_len must be the block size of the cipher.
+ */
+MBEDTLS_STATIC_TESTABLE int mbedtls_get_pkcs_padding(unsigned char *input,
+ size_t input_len,
+ size_t *data_len)
{
size_t i, pad_idx;
unsigned char padding_len;
@@ -849,10 +856,6 @@
}
padding_len = input[input_len - 1];
- if (padding_len == 0 || padding_len > input_len) {
- return MBEDTLS_ERR_CIPHER_INVALID_PADDING;
- }
- *data_len = input_len - padding_len;
mbedtls_ct_condition_t bad = mbedtls_ct_uint_gt(padding_len, input_len);
bad = mbedtls_ct_bool_or(bad, mbedtls_ct_uint_eq(padding_len, 0));
@@ -866,6 +869,9 @@
bad = mbedtls_ct_bool_or(bad, mbedtls_ct_bool_and(in_padding, different));
}
+ /* If the padding is invalid, set the output length to 0 */
+ *data_len = mbedtls_ct_if(bad, 0, input_len - padding_len);
+
return mbedtls_ct_error_if_else_0(bad, MBEDTLS_ERR_CIPHER_INVALID_PADDING);
}
#endif /* MBEDTLS_CIPHER_PADDING_PKCS7 */
@@ -1144,7 +1150,7 @@
#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
case MBEDTLS_PADDING_PKCS7:
ctx->add_padding = add_pkcs_padding;
- ctx->get_padding = get_pkcs_padding;
+ ctx->get_padding = mbedtls_get_pkcs_padding;
break;
#endif
#if defined(MBEDTLS_CIPHER_PADDING_ONE_AND_ZEROS)
diff --git a/library/cipher_invasive.h b/library/cipher_invasive.h
new file mode 100644
index 0000000..702f8f7
--- /dev/null
+++ b/library/cipher_invasive.h
@@ -0,0 +1,27 @@
+/**
+ * \file cipher_invasive.h
+ *
+ * \brief Cipher module: interfaces for invasive testing only.
+ *
+ * The interfaces in this file are intended for testing purposes only.
+ * They SHOULD NOT be made available in library integrations except when
+ * building the library for testing.
+ */
+/*
+ * Copyright The Mbed TLS Contributors
+ * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+ */
+#ifndef MBEDTLS_CIPHER_INVASIVE_H
+#define MBEDTLS_CIPHER_INVASIVE_H
+
+#include "common.h"
+
+#if defined(MBEDTLS_TEST_HOOKS) && defined(MBEDTLS_CIPHER_C)
+
+MBEDTLS_STATIC_TESTABLE int mbedtls_get_pkcs_padding(unsigned char *input,
+ size_t input_len,
+ size_t *data_len);
+
+#endif
+
+#endif /* MBEDTLS_CIPHER_INVASIVE_H */
diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function
index 040c35c..2856ae5 100644
--- a/tests/suites/test_suite_cipher.function
+++ b/tests/suites/test_suite_cipher.function
@@ -6,6 +6,10 @@
#include "mbedtls/gcm.h"
#endif
+#include "cipher_invasive.h"
+
+#include "test/constant_flow.h"
+
#if defined(MBEDTLS_CIPHER_HAVE_SOME_AEAD_VIA_LEGACY_OR_USE_PSA) || defined(MBEDTLS_NIST_KW_C)
#define MBEDTLS_CIPHER_AUTH_CRYPT
#endif
@@ -1260,3 +1264,20 @@
mbedtls_free(key);
}
/* END_CASE */
+
+/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS */
+void get_pkcs_padding(data_t *decrypted_block, int exp_ret, int exp_len)
+{
+ int ret;
+ size_t calculated_len;
+
+ TEST_CF_SECRET(decrypted_block->x, decrypted_block->len);
+ ret = mbedtls_get_pkcs_padding(decrypted_block->x, decrypted_block->len,
+ &calculated_len);
+
+ TEST_EQUAL(ret, exp_ret);
+ if (exp_ret == 0) {
+ TEST_EQUAL(calculated_len, exp_len);
+ }
+}
+/* END_CASE */
diff --git a/tests/suites/test_suite_cipher.padding.data b/tests/suites/test_suite_cipher.padding.data
index 0370fb3..85b14c1 100644
--- a/tests/suites/test_suite_cipher.padding.data
+++ b/tests/suites/test_suite_cipher.padding.data
@@ -217,3 +217,18 @@
Check no padding #3 (correct by definition)
check_padding:MBEDTLS_PADDING_NONE:"":0:0
+
+Constant-time PKCS7 padding, valid #1
+get_pkcs_padding:"00112233445566778899AABBCCDDEE01":0:15
+
+Constant-time PKCS7 padding, valid #2
+get_pkcs_padding:"00112233445566778899AA0505050505":0:11
+
+Constant-time PKCS7 padding, valid #3
+get_pkcs_padding:"10101010101010101010101010101010":0:0
+
+Constant-time PKCS7 padding, invalid zero
+get_pkcs_padding:"00112233445566778899AABBCCDDEE00":MBEDTLS_ERR_CIPHER_INVALID_PADDING:0
+
+Constant-time PKCS7 padding, invalid > 16
+get_pkcs_padding:"00112233445566778899AABBCCDDEE11":MBEDTLS_ERR_CIPHER_INVALID_PADDING:0