From 78b823ab0e8e6058927ca11a85557eaf3dcfe542 Mon Sep 17 00:00:00 2001 From: "Aaron M. Ucko" Date: Tue, 31 Jan 2023 15:45:44 -0500 Subject: [PATCH] mbedtls_mpi_sub_abs: Skip memcpy when redundant (#6701). In some contexts, the output pointer may equal the first input pointer, in which case copying is not only superfluous but results in "Source and destination overlap in memcpy" errors from Valgrind (as I observed in the context of ecp_double_jac) and a diagnostic message from TrustInSoft Analyzer (as Pascal Cuoq reported in the context of other ECP functions called by cert-app with a suitable certificate). Signed-off-by: Aaron M. Ucko --- ChangeLog.d/conditionalize-mbedtls_mpi_sub_abs-memcpy.txt | 4 ++++ library/bignum.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 ChangeLog.d/conditionalize-mbedtls_mpi_sub_abs-memcpy.txt diff --git a/ChangeLog.d/conditionalize-mbedtls_mpi_sub_abs-memcpy.txt b/ChangeLog.d/conditionalize-mbedtls_mpi_sub_abs-memcpy.txt new file mode 100644 index 000000000000..0a90721eafec --- /dev/null +++ b/ChangeLog.d/conditionalize-mbedtls_mpi_sub_abs-memcpy.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix potential undefined behavior in mbedtls_mpi_sub_abs(). Reported by + Pascal Cuoq using TrustInSoft Analyzer in #6701; observed independently by + Aaron Ucko under Valgrind. diff --git a/library/bignum.c b/library/bignum.c index 5ec0541e84a8..699d2c6de7a0 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1280,7 +1280,7 @@ int mbedtls_mpi_sub_abs(mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi /* Set the high limbs of X to match A. Don't touch the lower limbs * because X might be aliased to B, and we must not overwrite the * significant digits of B. */ - if (A->n > n) { + if (A->n > n && A != X) { memcpy(X->p + n, A->p + n, (A->n - n) * ciL); } if (X->n > A->n) {