Skip to content
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

URGENT: Stack buffer overflow verifying X.509 certificate #26

Closed
taviso opened this issue Feb 15, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@taviso
Copy link

commented Feb 15, 2019

Hello, while auditing some code using the MatrixSSL library (currently sold as the Inside Secure TLS Toolkit, previously also called GUARD TLS Toolkit), I happened to notice that a public X.509 certificate testcase for CVE-2014-1569 caused a stack buffer overflow.

I cleaned up the testcase a bit, to make a better demonstration. You can test it with the certValidate tool that comes with MatrixSSL.

$ gdb -q --args matrixssl/matrixssl/test/certValidate stackbufferoverflow.pem
Reading symbols from matrixssl/matrixssl/test/certValidate...done.
(gdb) r
Starting program: matrixssl/matrixssl/test/certValidate stackbufferoverflow.pem
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
  Loaded chain file stackbufferoverflow.pem
        [0]:berserk.filippo.io
        [1]:(null)
WARN subject not provided, SUBJ validation will be skipped

Program received signal SIGSEGV, Segmentation fault.
0x00005555555c5164 in pubRsaDecryptSignedElementExt
(gdb) bt
#0  0x00005555555c5164 in pubRsaDecryptSignedElementExt
#1  0x4141414141414141 in ?? ()
#2  0x0000000000000000 in ?? ()

I believe any client or server that validates certificates will be affected by this, and as MatrixSSL is usually used in embedded devices where mitigations are usually not quite as thorough as modern distributions, exploitation might not be difficult.

The bug is that pubRsaDecryptSignedElementExt() uses a fixed size stack buffer, but then doesn't check if the key size exceeds it. The patch below should solve it.

diff --git a/crypto/pubkey/rsa_pub.c b/crypto/pubkey/rsa_pub.c
index f1d57e0..fa36e42 100644
--- a/crypto/pubkey/rsa_pub.c
+++ b/crypto/pubkey/rsa_pub.c
@@ -63,6 +63,12 @@ int32_t psRsaDecryptPubExt(psPool_t *pool,
         return PS_ARG_FAIL;
     }
 
+    if (*outlen < key->size)
+    {
+        psTraceCrypto("Error on bad outlen parameter to psRsaDecryptPub\n");
+        return PS_ARG_FAIL;
+    }
+
     ptLen = inlen;
 
     /* Raw, in-place RSA decryption. */

Testcase:

stackbufferoverflow.pem

@matrixssl-admin

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2019

Thank you for noticing and reporting this issue. The analysis was good and we appreciate the suggested patch even if we decided to take a different approach. The attached testcase was really useful in reproducing the bug and verifying the fix.

The issue should now be addressed. Thanks again.

@taviso

This comment has been minimized.

Copy link
Author

commented Feb 20, 2019

Hello, just to be clear, the 4.0.2 release does not appear to contain the fix, is that intentional?

Thanks!

@matrixssl-admin

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2019

Appears that the fix was unintentionally left out of the commit. Should be fixed now. Sorry about the hassle and thanks for noticing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.