x509_get_subject copies 1 byte too many

[gvranken]

286   BIO_get_mem_ptr (subject_bio, &subject_mem);
287 
288   maxlen = subject_mem->length + 1;
289   subject = gc_malloc (maxlen, false, gc);
290 
291   memcpy (subject, subject_mem->data, maxlen);
292   subject[maxlen - 1] = '\0';

This copies 1 byte too many from OpenSSL memory. Please change it into:

286   BIO_get_mem_ptr (subject_bio, &subject_mem);
287 
288   maxlen = subject_mem->length;
289   subject = gc_malloc (maxlen+1, false, gc);
290 
291   memcpy (subject, subject_mem->data, maxlen);
292   subject[maxlen] = '\0';

[Steffan Karger]

Suggested change makes sense, will do.

Thanks for reporting.

(For future reference: since the extra copied byte is immediately overwritten with \0, this overread has no further security impact.)

[Steffan Karger]

Since you basically already provided a patch, would you not be interested in sending your patch to the list yourself? Just create a git commit out of it, and do git send-email --to=openvpn-devel@lists.sourceforge.net HEAD~1. That way you'll be properly attributed as the author of the patch.

[Gert Döring]

2.3 patch is in tree:

commit 04c84548c2e1f3ef55b6737b454ba5b4d903b319 (release/2.3)
Author: Guido Vranken
Date: Sat May 13 12:37:50 2017 +0200

Avoid a 1 byte overcopy in x509_get_subject (ssl_verify_openssl.c)

2.4/master:

commit 3fbc9d2b1b1e75b227107057b92ce6b786b5bea1 (master)
commit a91c54de67312f4026db61fd2b0e98cbd11e5323 (release/2.4)
Author: Steffan Karger
Date: Sun May 14 21:00:41 2017 +0200

Avoid a 1 byte overcopy in x509_get_subject (ssl_verify_openssl.c)

thanks!