Re: libcrypto: stop some ASN.1 bleeding
As promised, here is a new diff. Bob Beck suggested introducing wrappers
to the time-related functions, so that the error path becomes easier to
understand; this makes the diff to these functions much simpler indeed.
Index: a_gentm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_gentm.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_gentm.c
--- a_gentm.c 19 Apr 2014 11:43:07 - 1.17
+++ a_gentm.c 15 May 2014 18:30:29 -
@@ -208,20 +208,15 @@ ASN1_GENERALIZEDTIME_set(ASN1_GENERALIZE
return ASN1_GENERALIZEDTIME_adj(s, t, 0, 0);
}
-ASN1_GENERALIZEDTIME *
-ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
-long offset_sec)
+static ASN1_GENERALIZEDTIME *
+ASN1_GENERALIZEDTIME_adj_internal(ASN1_GENERALIZEDTIME *s, time_t t,
+int offset_day, long offset_sec)
{
char *p;
struct tm *ts;
struct tm data;
size_t len = 20;
- if (s == NULL)
- s = M_ASN1_GENERALIZEDTIME_new();
- if (s == NULL)
- return (NULL);
-
ts = gmtime_r(t, data);
if (ts == NULL)
return (NULL);
@@ -249,4 +244,25 @@ ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZE
s-length = strlen(p);
s-type = V_ASN1_GENERALIZEDTIME;
return (s);
+}
+
+ASN1_GENERALIZEDTIME *
+ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
+long offset_sec)
+{
+ ASN1_GENERALIZEDTIME *tmp = NULL, *ret;
+
+ if (s == NULL) {
+ tmp = M_ASN1_GENERALIZEDTIME_new();
+ if (tmp == NULL)
+ return NULL;
+ s = tmp;
+ }
+
+ ret = ASN1_GENERALIZEDTIME_adj_internal(s, t, offset_day, offset_sec);
+ if (ret == NULL tmp != NULL)
+ M_ASN1_GENERALIZEDTIME_free(tmp);
+
+ return ret;
+
}
Index: a_time.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_time.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_time.c
--- a_time.c21 Apr 2014 00:52:00 - 1.17
+++ a_time.c15 May 2014 18:30:29 -
@@ -120,8 +120,8 @@ ASN1_TIME_check(ASN1_TIME *t)
}
/* Convert an ASN1_TIME structure to GeneralizedTime */
-ASN1_GENERALIZEDTIME *
-ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
+static ASN1_GENERALIZEDTIME *
+ASN1_TIME_to_generalizedtime_internal(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
{
ASN1_GENERALIZEDTIME *ret;
char *str;
@@ -131,13 +131,7 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
if (!ASN1_TIME_check(t))
return NULL;
- if (!out || !*out) {
- if (!(ret = ASN1_GENERALIZEDTIME_new ()))
- return NULL;
- if (out)
- *out = ret;
- } else
- ret = *out;
+ ret = *out;
/* If already GeneralizedTime just copy across */
if (t-type == V_ASN1_GENERALIZEDTIME) {
@@ -152,12 +146,32 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
/* ASN1_STRING_set() allocated 'len + 1' bytes. */
newlen = t-length + 2 + 1;
str = (char *)ret-data;
+ /* XXX ASN1_TIME is not Y2050 compatible */
i = snprintf(str, newlen, %s%s, (t-data[0] = '5') ? 19 : 20,
(char *) t-data);
if (i == -1 || i = newlen) {
- ASN1_STRING_free(ret);
+ M_ASN1_GENERALIZEDTIME_free(ret);
+ *out = NULL;
return NULL;
}
+ return ret;
+}
+
+ASN1_GENERALIZEDTIME *
+ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
+{
+ ASN1_GENERALIZEDTIME *tmp = NULL, *ret;
+
+ if (!out || !*out) {
+ if (!(tmp = ASN1_GENERALIZEDTIME_new()))
+ return NULL;
+ out = tmp;
+ }
+
+ ret = ASN1_TIME_to_generalizedtime_internal(t, out);
+ if (ret == NULL tmp != NULL)
+ ASN1_GENERALIZEDTIME_free(tmp);
+
return ret;
}
Index: a_utctm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_utctm.c,v
retrieving revision 1.22
diff -u -p -r1.22 a_utctm.c
--- a_utctm.c 21 Apr 2014 11:23:09 - 1.22
+++ a_utctm.c 15 May 2014 18:30:29 -
@@ -149,19 +149,15 @@ ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t
return ASN1_UTCTIME_adj(s, t, 0, 0);
}
-ASN1_UTCTIME *
-ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t, int offset_day, long offset_sec)
+static ASN1_UTCTIME *
+ASN1_UTCTIME_adj_internal(ASN1_UTCTIME *s, time_t t, int offset_day,
+long offset_sec)
{
char *p;
struct tm *ts;
struct tm data;
size_t len = 20;
- if (s == NULL)
- s = M_ASN1_UTCTIME_new();
- if (s == NULL)
- return (NULL);
-
ts = gmtime_r(t, data);
if (ts == NULL)
Re: libcrypto: stop some ASN.1 bleeding
Miod Vallat m...@online.fr writes:
As promised, here is a new diff. Bob Beck suggested introducing wrappers
to the time-related functions, so that the error path becomes easier to
understand; this makes the diff to these functions much simpler
indeed.
That's helpful. It definitely makes it easier to read.
Index: a_gentm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_gentm.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_gentm.c
--- a_gentm.c 19 Apr 2014 11:43:07 - 1.17
+++ a_gentm.c 15 May 2014 18:30:29 -
@@ -208,20 +208,15 @@ ASN1_GENERALIZEDTIME_set(ASN1_GENERALIZE
return ASN1_GENERALIZEDTIME_adj(s, t, 0, 0);
}
-ASN1_GENERALIZEDTIME *
-ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
-long offset_sec)
+static ASN1_GENERALIZEDTIME *
+ASN1_GENERALIZEDTIME_adj_internal(ASN1_GENERALIZEDTIME *s, time_t t,
+int offset_day, long offset_sec)
{
char *p;
struct tm *ts;
struct tm data;
size_t len = 20;
- if (s == NULL)
- s = M_ASN1_GENERALIZEDTIME_new();
- if (s == NULL)
- return (NULL);
-
ts = gmtime_r(t, data);
if (ts == NULL)
return (NULL);
@@ -249,4 +244,25 @@ ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZE
s-length = strlen(p);
s-type = V_ASN1_GENERALIZEDTIME;
return (s);
+}
+
+ASN1_GENERALIZEDTIME *
+ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
+long offset_sec)
+{
+ ASN1_GENERALIZEDTIME *tmp = NULL, *ret;
+
+ if (s == NULL) {
+ tmp = M_ASN1_GENERALIZEDTIME_new();
+ if (tmp == NULL)
+ return NULL;
+ s = tmp;
+ }
+
+ ret = ASN1_GENERALIZEDTIME_adj_internal(s, t, offset_day, offset_sec);
+ if (ret == NULL tmp != NULL)
+ M_ASN1_GENERALIZEDTIME_free(tmp);
+
+ return ret;
+
}
Index: a_time.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_time.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_time.c
--- a_time.c 21 Apr 2014 00:52:00 - 1.17
+++ a_time.c 15 May 2014 18:30:29 -
@@ -120,8 +120,8 @@ ASN1_TIME_check(ASN1_TIME *t)
}
/* Convert an ASN1_TIME structure to GeneralizedTime */
-ASN1_GENERALIZEDTIME *
-ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
+static ASN1_GENERALIZEDTIME *
+ASN1_TIME_to_generalizedtime_internal(ASN1_TIME *t, ASN1_GENERALIZEDTIME
**out)
{
ASN1_GENERALIZEDTIME *ret;
char *str;
@@ -131,13 +131,7 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
if (!ASN1_TIME_check(t))
return NULL;
- if (!out || !*out) {
- if (!(ret = ASN1_GENERALIZEDTIME_new ()))
- return NULL;
- if (out)
- *out = ret;
- } else
- ret = *out;
+ ret = *out;
/* If already GeneralizedTime just copy across */
if (t-type == V_ASN1_GENERALIZEDTIME) {
@@ -152,12 +146,32 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
/* ASN1_STRING_set() allocated 'len + 1' bytes. */
newlen = t-length + 2 + 1;
str = (char *)ret-data;
+ /* XXX ASN1_TIME is not Y2050 compatible */
i = snprintf(str, newlen, %s%s, (t-data[0] = '5') ? 19 : 20,
(char *) t-data);
if (i == -1 || i = newlen) {
- ASN1_STRING_free(ret);
+ M_ASN1_GENERALIZEDTIME_free(ret);
+ *out = NULL;
return NULL;
}
+ return ret;
+}
+
+ASN1_GENERALIZEDTIME *
+ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
+{
+ ASN1_GENERALIZEDTIME *tmp = NULL, *ret;
+
+ if (!out || !*out) {
+ if (!(tmp = ASN1_GENERALIZEDTIME_new()))
+ return NULL;
+ out = tmp;
So we no longer return the newly-allocated ASN1_GENERALIZEDTIME through
*out? This might be a problem, since callers seem to use the value in
*out instead of storing the return value.
+ }
+
+ ret = ASN1_TIME_to_generalizedtime_internal(t, out);
+ if (ret == NULL tmp != NULL)
+ ASN1_GENERALIZEDTIME_free(tmp);
+
return ret;
}
Index: a_utctm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_utctm.c,v
retrieving revision 1.22
diff -u -p -r1.22 a_utctm.c
--- a_utctm.c 21 Apr 2014 11:23:09 - 1.22
+++ a_utctm.c 15 May 2014 18:30:29 -
@@ -149,19 +149,15 @@ ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t
return ASN1_UTCTIME_adj(s, t, 0, 0);
}
-ASN1_UTCTIME *
-ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t, int offset_day, long offset_sec)
+static ASN1_UTCTIME *
+ASN1_UTCTIME_adj_internal(ASN1_UTCTIME *s, time_t t, int offset_day,
+long
Re: libcrypto: stop some ASN.1 bleeding
Index: a_time.c
+ASN1_GENERALIZEDTIME *
+ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
+{
+ ASN1_GENERALIZEDTIME *tmp = NULL, *ret;
+
+ if (!out || !*out) {
+ if (!(tmp = ASN1_GENERALIZEDTIME_new()))
+ return NULL;
+ out = tmp;
So we no longer return the newly-allocated ASN1_GENERALIZEDTIME through
*out? This might be a problem, since callers seem to use the value in
*out instead of storing the return value.
Oh, you mean in the case where out != NULL but *out == NULL. You're
right. The last line above should be
if (out != NULL)
*out = tmp;
else
out = tmp;
Fixed diff below.
Index: a_gentm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_gentm.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_gentm.c
--- a_gentm.c 19 Apr 2014 11:43:07 - 1.17
+++ a_gentm.c 15 May 2014 19:34:46 -
@@ -208,20 +208,15 @@ ASN1_GENERALIZEDTIME_set(ASN1_GENERALIZE
return ASN1_GENERALIZEDTIME_adj(s, t, 0, 0);
}
-ASN1_GENERALIZEDTIME *
-ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
-long offset_sec)
+static ASN1_GENERALIZEDTIME *
+ASN1_GENERALIZEDTIME_adj_internal(ASN1_GENERALIZEDTIME *s, time_t t,
+int offset_day, long offset_sec)
{
char *p;
struct tm *ts;
struct tm data;
size_t len = 20;
- if (s == NULL)
- s = M_ASN1_GENERALIZEDTIME_new();
- if (s == NULL)
- return (NULL);
-
ts = gmtime_r(t, data);
if (ts == NULL)
return (NULL);
@@ -249,4 +244,25 @@ ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZE
s-length = strlen(p);
s-type = V_ASN1_GENERALIZEDTIME;
return (s);
+}
+
+ASN1_GENERALIZEDTIME *
+ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
+long offset_sec)
+{
+ ASN1_GENERALIZEDTIME *tmp = NULL, *ret;
+
+ if (s == NULL) {
+ tmp = M_ASN1_GENERALIZEDTIME_new();
+ if (tmp == NULL)
+ return NULL;
+ s = tmp;
+ }
+
+ ret = ASN1_GENERALIZEDTIME_adj_internal(s, t, offset_day, offset_sec);
+ if (ret == NULL tmp != NULL)
+ M_ASN1_GENERALIZEDTIME_free(tmp);
+
+ return ret;
+
}
Index: a_time.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_time.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_time.c
--- a_time.c21 Apr 2014 00:52:00 - 1.17
+++ a_time.c15 May 2014 19:34:46 -
@@ -120,8 +120,8 @@ ASN1_TIME_check(ASN1_TIME *t)
}
/* Convert an ASN1_TIME structure to GeneralizedTime */
-ASN1_GENERALIZEDTIME *
-ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
+static ASN1_GENERALIZEDTIME *
+ASN1_TIME_to_generalizedtime_internal(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
{
ASN1_GENERALIZEDTIME *ret;
char *str;
@@ -131,13 +131,7 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
if (!ASN1_TIME_check(t))
return NULL;
- if (!out || !*out) {
- if (!(ret = ASN1_GENERALIZEDTIME_new ()))
- return NULL;
- if (out)
- *out = ret;
- } else
- ret = *out;
+ ret = *out;
/* If already GeneralizedTime just copy across */
if (t-type == V_ASN1_GENERALIZEDTIME) {
@@ -152,12 +146,35 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
/* ASN1_STRING_set() allocated 'len + 1' bytes. */
newlen = t-length + 2 + 1;
str = (char *)ret-data;
+ /* XXX ASN1_TIME is not Y2050 compatible */
i = snprintf(str, newlen, %s%s, (t-data[0] = '5') ? 19 : 20,
(char *) t-data);
if (i == -1 || i = newlen) {
- ASN1_STRING_free(ret);
+ M_ASN1_GENERALIZEDTIME_free(ret);
+ *out = NULL;
return NULL;
}
+ return ret;
+}
+
+ASN1_GENERALIZEDTIME *
+ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
+{
+ ASN1_GENERALIZEDTIME *tmp = NULL, *ret;
+
+ if (!out || !*out) {
+ if (!(tmp = ASN1_GENERALIZEDTIME_new()))
+ return NULL;
+ if (out != NULL)
+ *out = tmp;
+ else
+ out = tmp;
+ }
+
+ ret = ASN1_TIME_to_generalizedtime_internal(t, out);
+ if (ret == NULL tmp != NULL)
+ ASN1_GENERALIZEDTIME_free(tmp);
+
return ret;
}
Index: a_utctm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_utctm.c,v
retrieving revision 1.22
diff -u -p -r1.22 a_utctm.c
--- a_utctm.c 21 Apr 2014 11:23:09 - 1.22
+++
Re: libcrypto: stop some ASN.1 bleeding
OK, the latest diff looks good to me.
libcrypto: stop some ASN.1 bleeding
... or, in other words, try to fix most memory leak upon failure.
This kind of change is difficult to test, the more eyes reviewing it,
the better.
Miod
Index: a_gentm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_gentm.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_gentm.c
--- a_gentm.c 19 Apr 2014 11:43:07 - 1.17
+++ a_gentm.c 14 May 2014 21:18:03 -
@@ -212,41 +212,48 @@ ASN1_GENERALIZEDTIME *
ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
long offset_sec)
{
+ ASN1_GENERALIZEDTIME *ret;
char *p;
struct tm *ts;
struct tm data;
size_t len = 20;
- if (s == NULL)
- s = M_ASN1_GENERALIZEDTIME_new();
- if (s == NULL)
- return (NULL);
+ if (s == NULL) {
+ ret = M_ASN1_GENERALIZEDTIME_new();
+ if (ret == NULL)
+ return (NULL);
+ } else
+ ret = s;
ts = gmtime_r(t, data);
if (ts == NULL)
- return (NULL);
+ goto err;
if (offset_day || offset_sec) {
if (!OPENSSL_gmtime_adj(ts, offset_day, offset_sec))
- return NULL;
+ goto err;
}
- p = (char *)s-data;
- if ((p == NULL) || ((size_t)s-length len)) {
+ p = (char *)ret-data;
+ if ((p == NULL) || ((size_t)ret-length len)) {
p = malloc(len);
if (p == NULL) {
ASN1err(ASN1_F_ASN1_GENERALIZEDTIME_ADJ,
ERR_R_MALLOC_FAILURE);
- return (NULL);
+ goto err;
}
- if (s-data != NULL)
- free(s-data);
- s-data = (unsigned char *)p;
+ if (ret-data != NULL)
+ free(ret-data);
+ ret-data = (unsigned char *)p;
}
snprintf(p, len, %04d%02d%02d%02d%02d%02dZ, ts-tm_year + 1900,
ts-tm_mon + 1, ts-tm_mday, ts-tm_hour, ts-tm_min, ts-tm_sec);
- s-length = strlen(p);
- s-type = V_ASN1_GENERALIZEDTIME;
- return (s);
+ ret-length = strlen(p);
+ ret-type = V_ASN1_GENERALIZEDTIME;
+ return (ret);
+err:
+ if (ret != s)
+ M_ASN1_GENERALIZEDTIME_free(ret);
+ return NULL;
}
Index: a_time.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_time.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_time.c
--- a_time.c21 Apr 2014 00:52:00 - 1.17
+++ a_time.c14 May 2014 21:18:03 -
@@ -123,7 +123,7 @@ ASN1_TIME_check(ASN1_TIME *t)
ASN1_GENERALIZEDTIME *
ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
{
- ASN1_GENERALIZEDTIME *ret;
+ ASN1_GENERALIZEDTIME *ret = NULL;
char *str;
int newlen;
int i;
@@ -132,33 +132,41 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
return NULL;
if (!out || !*out) {
- if (!(ret = ASN1_GENERALIZEDTIME_new ()))
+ if (!(ret = ASN1_GENERALIZEDTIME_new()))
return NULL;
- if (out)
- *out = ret;
} else
ret = *out;
/* If already GeneralizedTime just copy across */
if (t-type == V_ASN1_GENERALIZEDTIME) {
if (!ASN1_STRING_set(ret, t-data, t-length))
- return NULL;
- return ret;
+ goto err;
+ goto done;
}
/* grow the string */
if (!ASN1_STRING_set(ret, NULL, t-length + 2))
- return NULL;
+ goto err;
/* ASN1_STRING_set() allocated 'len + 1' bytes. */
newlen = t-length + 2 + 1;
str = (char *)ret-data;
+ /* XXX ASN1_TIME is not Y2050 compatible */
i = snprintf(str, newlen, %s%s, (t-data[0] = '5') ? 19 : 20,
(char *) t-data);
if (i == -1 || i = newlen) {
- ASN1_STRING_free(ret);
+ M_ASN1_GENERALIZEDTIME_free(ret);
+ if (out *out == ret)
+ *out = NULL;
return NULL;
}
+done:
+ if (out)
+ *out = ret;
return ret;
+err:
+ if (out != NULL ret != *out)
+ ASN1_GENERALIZEDTIME_free(ret);
+ return NULL;
}
int
Index: a_utctm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_utctm.c,v
retrieving revision 1.22
diff -u -p -r1.22 a_utctm.c
--- a_utctm.c 21 Apr 2014 11:23:09 - 1.22
+++ a_utctm.c 14 May 2014 21:18:03 -
@@ -152,34 +152,37 @@ ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t
ASN1_UTCTIME *
ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t
Re: libcrypto: stop some ASN.1 bleeding
Miod Vallat m...@online.fr writes:
... or, in other words, try to fix most memory leak upon failure.
This kind of change is difficult to test, the more eyes reviewing it,
the better.
Well, I'll try to take a stab at it then.
Miod
Index: a_gentm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_gentm.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_gentm.c
--- a_gentm.c 19 Apr 2014 11:43:07 - 1.17
+++ a_gentm.c 14 May 2014 21:18:03 -
@@ -212,41 +212,48 @@ ASN1_GENERALIZEDTIME *
ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
long offset_sec)
{
+ ASN1_GENERALIZEDTIME *ret;
char *p;
struct tm *ts;
struct tm data;
size_t len = 20;
- if (s == NULL)
- s = M_ASN1_GENERALIZEDTIME_new();
- if (s == NULL)
- return (NULL);
+ if (s == NULL) {
+ ret = M_ASN1_GENERALIZEDTIME_new();
+ if (ret == NULL)
+ return (NULL);
+ } else
+ ret = s;
ts = gmtime_r(t, data);
if (ts == NULL)
- return (NULL);
+ goto err;
if (offset_day || offset_sec) {
if (!OPENSSL_gmtime_adj(ts, offset_day, offset_sec))
- return NULL;
+ goto err;
}
- p = (char *)s-data;
- if ((p == NULL) || ((size_t)s-length len)) {
+ p = (char *)ret-data;
+ if ((p == NULL) || ((size_t)ret-length len)) {
p = malloc(len);
if (p == NULL) {
ASN1err(ASN1_F_ASN1_GENERALIZEDTIME_ADJ,
ERR_R_MALLOC_FAILURE);
- return (NULL);
+ goto err;
}
- if (s-data != NULL)
- free(s-data);
- s-data = (unsigned char *)p;
+ if (ret-data != NULL)
+ free(ret-data);
+ ret-data = (unsigned char *)p;
}
You might be able to use ASN1_STRING_set(ret, NULL, len) to handle
resizing / allocating the string the same way that it's used in
ASN1_TIME_to_generalizedtime(). Alternately, you might want to at least
use 'p = realloc(ret-data, len)' instead of malloc + free.
snprintf(p, len, %04d%02d%02d%02d%02d%02dZ, ts-tm_year + 1900,
ts-tm_mon + 1, ts-tm_mday, ts-tm_hour, ts-tm_min, ts-tm_sec);
- s-length = strlen(p);
- s-type = V_ASN1_GENERALIZEDTIME;
- return (s);
+ ret-length = strlen(p);
+ ret-type = V_ASN1_GENERALIZEDTIME;
+ return (ret);
+err:
+ if (ret != s)
+ M_ASN1_GENERALIZEDTIME_free(ret);
+ return NULL;
}
Index: a_time.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_time.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_time.c
--- a_time.c 21 Apr 2014 00:52:00 - 1.17
+++ a_time.c 14 May 2014 21:18:03 -
@@ -123,7 +123,7 @@ ASN1_TIME_check(ASN1_TIME *t)
ASN1_GENERALIZEDTIME *
ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
{
- ASN1_GENERALIZEDTIME *ret;
+ ASN1_GENERALIZEDTIME *ret = NULL;
char *str;
int newlen;
int i;
@@ -132,33 +132,41 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
return NULL;
if (!out || !*out) {
- if (!(ret = ASN1_GENERALIZEDTIME_new ()))
+ if (!(ret = ASN1_GENERALIZEDTIME_new()))
return NULL;
- if (out)
- *out = ret;
} else
ret = *out;
/* If already GeneralizedTime just copy across */
if (t-type == V_ASN1_GENERALIZEDTIME) {
if (!ASN1_STRING_set(ret, t-data, t-length))
- return NULL;
- return ret;
+ goto err;
+ goto done;
}
/* grow the string */
if (!ASN1_STRING_set(ret, NULL, t-length + 2))
- return NULL;
+ goto err;
/* ASN1_STRING_set() allocated 'len + 1' bytes. */
newlen = t-length + 2 + 1;
str = (char *)ret-data;
+ /* XXX ASN1_TIME is not Y2050 compatible */
i = snprintf(str, newlen, %s%s, (t-data[0] = '5') ? 19 : 20,
(char *) t-data);
if (i == -1 || i = newlen) {
- ASN1_STRING_free(ret);
+ M_ASN1_GENERALIZEDTIME_free(ret);
+ if (out *out == ret)
+ *out = NULL;
return NULL;
}
The fact that *out can be released here seems a bit unexpected, but I
can't figure out how to get i = newlen for this case anyway.
+done:
+ if (out)
+ *out = ret;
return ret;
+err:
+ if (out != NULL ret != *out)
+ ASN1_GENERALIZEDTIME_free(ret);
+ return NULL;
}
int
Re: libcrypto: stop some ASN.1 bleeding
... or, in other words, try to fix most memory leak upon failure. This kind of change is difficult to test, the more eyes reviewing it, the better. Well, I'll try to take a stab at it then. You might be able to use ASN1_STRING_set(ret, NULL, len) to handle resizing / allocating the string the same way that it's used in ASN1_TIME_to_generalizedtime(). Alternately, you might want to at least use 'p = realloc(ret-data, len)' instead of malloc + free. I'm trying to stick to the existing logic and only plug the memory leaks here. Code factoring can happen as a later diff (in other words: I'm writing this down on the list). Looks like there's a bit of an oversight here -- ret isn't referred to after this point in the function. Oops, you're right. I'll send an updated diff this evening. Miod
