Rework ASN1_STRING_set()

Rework ASN1_STRING_set() so that we always clear and free an existing
allocation, prior to storing the new data. This fixes a number of issues,
including a failure to zero data if the existing allocation was too small.
This also fixes other bugs such as leaving the allocation uninitialised
if NULL is passed for data. Require -1 where strlen() is expected and
improve length and overflow checks.

ok inoguchi@ tb@

1 files changed, 21 insertions, 14 deletions

diff --git a/src/lib/libcrypto/asn1/a_string.c b/src/lib/libcrypto/asn1/a_string.c
index 217d28da09..90e363e9c7 100644
--- a/src/lib/libcrypto/asn1/a_string.c
+++ b/src/lib/libcrypto/asn1/a_string.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_string.c,v 1.6 2022/03/14 16:35:45 jsing Exp $ */
+/* $OpenBSD: a_string.c,v 1.7 2022/03/17 17:17:58 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -159,26 +159,33 @@ ASN1_STRING_set(ASN1_STRING *astr, const void *_data, int len)
 {
         const char *data = _data;
 
-        if (len < 0) {
+        if (len == -1) {
+                size_t slen;
+
                 if (data == NULL)
                         return 0;
-                else
-                        len = strlen(data);
-        }
-        if ((astr->length < len) || (astr->data == NULL)) {
-                unsigned char *tmp;
-                tmp = realloc(astr->data, len + 1);
-                if (tmp == NULL) {
-                        ASN1error(ERR_R_MALLOC_FAILURE);
+
+                if ((slen = strlen(data)) > INT_MAX)
                         return 0;
-                }
-                astr->data = tmp;
+
+                len = (int)slen;
+        }
+
+        ASN1_STRING_clear(astr);
+
+        if (len < 0 || len >= INT_MAX)
+                return 0;
+
+        if ((astr->data = calloc(1, len + 1)) == NULL) {
+                ASN1error(ERR_R_MALLOC_FAILURE);
+                return (0);
         }
         astr->length = len;
+
         if (data != NULL) {
-                memmove(astr->data, data, len);
+                memcpy(astr->data, data, len);
+                astr->data[len] = '\0';
         }
-        astr->data[astr->length] = '\0';
 
         return 1;
 }