-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
memcpy on overlapping memory zones when verifying a certificate #6701
Labels
bug
component-crypto
Crypto primitives and low-level interfaces
size-s
Estimated task size: small (~2d)
Comments
Interestingly, as of Clang 15, Asan yells with a partially overlapping |
gilles-peskine-arm
added
bug
component-crypto
Crypto primitives and low-level interfaces
size-s
Estimated task size: small (~2d)
labels
Dec 1, 2022
3 tasks
ucko
added a commit
to ucko/mbedtls
that referenced
this issue
Jan 17, 2023
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 <ucko@ncbi.nlm.nih.gov>
gilles-peskine-arm
added a commit
that referenced
this issue
Feb 1, 2023
mbedtls_mpi_sub_abs: Skip memcpy when redundant (#6701).
gilles-peskine-arm
pushed a commit
that referenced
this issue
Feb 1, 2023
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 <ucko@ncbi.nlm.nih.gov>
gilles-peskine-arm
added a commit
that referenced
this issue
Feb 1, 2023
[Backport 2.28] mbedtls_mpi_sub_abs: Skip memcpy when redundant (#6701).
yanrayw
pushed a commit
to yanrayw/mbedtls
that referenced
this issue
Feb 7, 2023
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 <ucko@ncbi.nlm.nih.gov>
paul-elliott-arm
pushed a commit
to paul-elliott-arm/mbedtls
that referenced
this issue
Feb 14, 2023
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 <ucko@ncbi.nlm.nih.gov>
gouriano
pushed a commit
to ncbi/ncbi-cxx-toolkit-public
that referenced
this issue
May 25, 2023
"Source and destination overlap in memcpy" would normally be a legitimate cause for concern, but is a harmless formality in this case because they're consistently identical, as detailed in Mbed-TLS/mbedtls#6701 (whose fix VDB evidently doesn't yet bundle). JIRA: CXX-13024. git-svn-id: https://anonsvn.ncbi.nlm.nih.gov/repos/v1/trunk/c++@99869 78c7ea69-d796-4a43-9a09-de51944f1b03
chenxuuu
pushed a commit
to openLuat/LuatOS
that referenced
this issue
Oct 29, 2023
lhuang04
pushed a commit
to lhuang04/mbedtls
that referenced
this issue
Apr 2, 2024
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 <ucko@ncbi.nlm.nih.gov>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
component-crypto
Crypto primitives and low-level interfaces
size-s
Estimated task size: small (~2d)
Summary
When
cert-app
is invoked with the provided certificate, the functionmbedtls_mpi_sub_abs
eventually callsmemcpy
on identical, and thus overlapping, memory zones.mbedtls/library/bignum.c
Line 964 in a942b37
Both
X
andA
are pointing toYY
, local variable of the functionecp_check_pubkey_sw
. The callstack at the time of the invalidmemcpy
call is:System information
Mbed TLS version (number or commit id): commit a942b37
Operating system and version: abstract POSIX system with implementation-defined choices of GCC for IA32
Configuration (if not default, please attach
mbedtls_config.h
):mbedtls_config.h
from the repo.Compiler and options (if you used a pre-built binary, please indicate how you obtained it): abstract compiler with the implementation-defined choices of GCC targeting IA32.
Expected behavior
The specification of the standard function
memcpy
says that, unlikememmove
, it should never be called on overlapping memory zones.Actual behavior
memcpy
is invoked on identical pointer arguments and a nonzero size (36), therefore the source and destination memory zones overlap.Steps to reproduce
Invoke
cert-app
with the commandlinemode=file
and provide the following certificate ascert.crt
:Additional information
I follow the development of enough open-source projects to be familiar with the usual reports from well-meaning users of the project who have applied a static analyzer to the source code without being able to do the classification of warnings between false positives and bugs. I would like to emphasize that this is not such a report: the situation I described certainly happens for the provided input.
This bug was found using TrustInSoft Analyzer during the REDOCS'22 event.
The text was updated successfully, but these errors were encountered: