memcpy on overlapping memory zones when verifying a certificate

[pascal-cuoq]

Summary

When cert-app is invoked with the provided certificate, the function mbedtls_mpi_sub_abs eventually calls memcpy on identical, and thus overlapping, memory zones.

mbedtls/library/bignum.c

Line 964 in a942b37

memcpy( X->p + n, A->p + n, ( A->n - n ) * ciL );

Both X and A are pointing to YY, local variable of the function ecp_check_pubkey_sw. The callstack at the time of the invalid memcpy call is:

                  stack: memcpy :: library/bignum.c:964 <-
                         mbedtls_mpi_sub_abs :: library/bignum.c:1007 <-
                         add_sub_mpi :: library/bignum.c:1036 <-
                         mbedtls_mpi_add_mpi :: library/ecp.c:984 <-
                         ecp_modp :: library/ecp.c:1027 <-
                         mbedtls_mpi_mul_mod :: library/ecp.c:2616 <-
                         ecp_check_pubkey_sw :: library/ecp.c:2922 <-
                         mbedtls_ecp_check_pubkey :: library/pkparse.c:485 <-
                         pk_get_ecpubkey :: library/pkparse.c:636 <-
                         mbedtls_pk_parse_subpubkey :: library/x509_crt.c:1237 <-
                         x509_crt_parse_der_core :: library/x509_crt.c:1372 <-
                         mbedtls_x509_crt_parse_der_internal :: library/x509_crt.c:1408 <-
                         mbedtls_x509_crt_parse_der :: library/x509_crt.c:1496 <-
                         mbedtls_x509_crt_parse :: library/x509_crt.c:1541 <-
                         mbedtls_x509_crt_parse_file :: programs/x509/cert_app.c:287 <-
                         main

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, unlike memmove, 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 commandline mode=file and provide the following certificate as cert.crt:

-----BEGIN CERTIFICATE-----
MIIFBDCCBK6gAwIBAgIQA3dcl4czgOpyESx05xx+6DANBgkqhkiG9w0BAQsFADAb
MRkwFwYDVQQDDBByZWRvY3MyMl9UaVMuY29tMB4XDTIyMTEyNzAwMDAwMFoXDTIz
MDEyNjAwMDAwMFowdTELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWEx
FjAUBgNVBAcTDVNhbiBGcmFuY2lzY28xGTAXBgNVBAoTEENsb3VkZmxhcmUsIElu
Yy4xHjAcBgNVBAMTFXNuaS5jbG91ZGZsYXJlc3NsLmNvbTBZMBMGByqGSM49AgEG
CCqGSM49AwEHA0IABLC2SYCX1B5yEHzW09W6GKBbmeDCeF19SKT8ajn3vatLFuNS
d1gS9jPVjKO+U/BI5O3RTIWFmXT9fUez8Ljmi6KjggN1MIIDcTAfBgNVHSMEGDAW
gBSlzjfq67B1DpRniLRF+tkkEIeWHzAdBgNVHQ4EFgQUJIDOCqUEKti9Q8U5Sr8/
dza0jT0wPAYDVR0RBDUwM4ILZGlzY29yZC5jb22CDSouZGlzY29yZC5jb22CFXNu
aS5jbG91ZGZsYXJlc3NsLmNvbTAOBgNVHQ8BAf8EBAMCB4AwHQYDVR0lBBYwFAYI
KwYBBQUHAwEGCCsGAQUFBwMCMHsGA1UdHwR0MHIwN6A1oDOGMWh0dHA6Ly9jcmwz
LmRpZ2ljZXJ0LmNvbS9DbG91ZGZsYXJlSW5jRUNDQ0EtMy5jcmwwN6A1oDOGMWh0
dHA6Ly9jcmw0LmRpZ2ljZXJ0LmNvbS9DbG91ZGZsYXJlSW5jRUNDQ0EtMy5jcmww
PgYDVR0gBDcwNTAzBgZngQwBAgIwKTAnBggrBgEFBQcCARYbaHR0cDovL3d3dy5k
aWdpY2VydC5jb20vQ1BTMHYGCCsGAQUFBwEBBGowaDAkBggrBgEFBQcwAYYYaHR0
cDovL29jc3AuZGlnaWNlcnQuY29tMEAGCCsGAQUFBzAChjRodHRwOi8vY2FjZXJ0
cy5kaWdpY2VydC5jb20vQ2xvdWRmbGFyZUluY0VDQ0NBLTMuY3J0MAwGA1UdEwEB
/wQCMAAwggF9BgorBgEEAdZ5AgQCBIIBbQSCAWkBZwB2AOg+0No+9QY1MudXKLyJ
a8kD08vREWvs62nhd31tBr1uAAABhI2kUosAAAQDAEcwRQIhAPXXVS3R1NwneuvB
5tHjLystYFlNMdjS8X0WeP07+mtoAiBq64+xNvTNVT63MrJc7WYFg5V1abJS81JN
zYyhzT4ukwB2ALNzdwfhhFD4Y4bWBancEQlKeS2xZwwLh9zwAw55NqWaAAABhI2k
UwEAAAQDAEcwRQIgdezKk7oA0SDKXnUKLLxtVDQl+VocRGo9/vQFoyexYGgCIQCH
vblbEQEgVcfwr3c7g5MJTUyXigLp6pAZ7LlVMyji5QB1ALc++yTfnE26dfI5xbpY
9Gxd/ELPep81xJ4dCYEl7bSZAAABhI2kUr0AAAQDAEYwRAIgKSHr31cDLiIvk4gp
BrYHZQ9C3ihBuguZreR4KorYydMCIBMFYMUMbO/JiLMnMsaEPUQJGT9hTchBvoMc
QZpEoncyMA0GCSqGSIb3DQEBCwUAA0EAF7eqdr95Tm1eFzclAcI79Z37DUll0pQ/
L2RIvRuChCBl4kGPIVy/vZycgXtPaHxT/Px3IJ15YyUsV6SCvn9KYw==
-----END CERTIFICATE-----

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.

[gilles-peskine-arm]

Interestingly, as of Clang 15, Asan yells with a partially overlapping memcpy, but not when the source is equal to the destination. So even though we do have a test case that does have the problematic behavior, the test is passing. Score another point for static analysis.

[tom-cosgrove-arm]

Duplicate of #4387, and fixed by #6942