From 50eb3bc245d98bcf4b386f3fc7db839c788d5be6 Mon Sep 17 00:00:00 2001
From: Nicolas Williams <nico@twosigma.com>
Date: Wed, 30 Nov 2022 10:48:48 -0600
Subject: [PATCH] asn1: Fix 1-byte leaks in der_copy_octet_string()

We sometimes do things like `memset(&cert, 0, sizeof(cert))` then
`copy_Certificate(&cert, &cert_copy)`, and then we end up leaking a
byte in `der_copy_octet_string()` due to it having this code:

```C
der_copy_octet_string (const heim_octet_string *from, heim_octet_string *to)
{
    assert(from->length == 0 || (from->length > 0 && from->data != NULL));
    if (from->length == 0)
	to->data = calloc(1, 1);
    else
	to->data = malloc(from->length);
    ...
}
```

The traces where this happens always involve the `_save` field of
`Name` or `TBSCertificate`.

This code was assuming that length 0 octet strings are expected to have
a non-NULL `data`, probably in case the C library's allocator returns
non-NULL pointers for `malloc(0)`, but then, why not just call
`malloc(0)`?  But calling `malloc(0)` would then still lead to this leak
in on such systems.

Now, `der_free_octet_string()` does unconditionally `free()` the
string's `data`, so the leak really is not there but elsewhere, probably
in `lib/asn1/template.c:_asn1_free()`, but it clearly does
`der_free_octet_string()` the `_save` field of types that have it.
---
 lib/asn1/der_copy.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/asn1/der_copy.c b/lib/asn1/der_copy.c
index 2084cef5f..f67fff69d 100644
--- a/lib/asn1/der_copy.c
+++ b/lib/asn1/der_copy.c
@@ -167,9 +167,13 @@ int ASN1CALL
 der_copy_octet_string (const heim_octet_string *from, heim_octet_string *to)
 {
     assert(from->length == 0 || (from->length > 0 && from->data != NULL));
-    if (from->length == 0)
+    if (from->length == 0) {
+        if (from->data == NULL) {
+            *to = *from;
+            return 0;
+        }
 	to->data = calloc(1, 1);
-    else
+    } else
 	to->data = malloc(from->length);
     if (to->data == NULL) {
 	to->length = 0;