On a Thursday in 2022, Jiri Denemark wrote:
If 1024 was not enough to fit the DN, gnutls_x509_crt_get_dn would store
the required size in subjectlen. And since we're not checking the return
value of this function, we would happily overwrite some random memory.
Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
---
src/qemu/qemu_migration_cookie.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 76a01781d6..046d588d8a 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -180,12 +180,12 @@ static char *
qemuDomainExtractTLSSubject(const char *certdir)
{
g_autofree char *certfile = NULL;
- char *subject = NULL;
+ g_autofree char *subject = NULL;
g_autofree char *pemdata = NULL;
gnutls_datum_t pemdatum;
gnutls_x509_crt_t cert;
int rc;
- size_t subjectlen;
+ size_t subjectlen = 0;
certfile = g_strdup_printf("%s/server-cert.pem", certdir);
@@ -214,13 +214,20 @@ qemuDomainExtractTLSSubject(const char *certdir)
return NULL;
}
- subjectlen = 1024;
- subject = g_new0(char, subjectlen + 1);
-
- gnutls_x509_crt_get_dn(cert, subject, &subjectlen);
+ rc = gnutls_x509_crt_get_dn(cert, NULL, &subjectlen);
Leaving the pre-allocation here would be more consistent with our other use of this function (where the original buffer is only 256 bytes long) Internally, the function formats the answer into its own buffer to figure out its size, so it would also serve as a micro-optimization.
+ if (rc == GNUTLS_E_SHORT_MEMORY_BUFFER) {
+ subject = g_new0(char, subjectlen + 1);
+ rc = gnutls_x509_crt_get_dn(cert, subject, &subjectlen);
+ }
+ if (rc != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot get cert distinguished name: %s"),
Please copy the translatable error string from either of the two other uses of gnutls_x509_crt_get_dn, to save the translators some work. Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx> Jano
+ gnutls_strerror(rc));
+ return NULL;
+ }
subject[subjectlen] = '\0';
- return subject;
+ return g_steal_pointer(&subject);
}
--
2.35.1
Attachment:
signature.asc
Description: PGP signature
