Re: libssl/src/apps don't cast {m,re}alloc

[Thomas Pfaff]

 This doesn't fix the problems, only removes markers alerting us
 to audit it.
 
 Memory management in these files is still missing integer overflow
 checks, NULL return checks, and is full of crazy abominations [...]

Yes, I saw that but I thought I'd take care of one thing first
then send patches to fix other things, but I get your point.

 X509_NAME *
 parse_name(char *subject, long chtype, int multirdn)
 {
   size_t buflen = strlen(subject) + 1;/* ...
   char *buf = malloc(buflen);
   size_t max_ne = buflen / 2 + 1; /* maximum number of name elements */
   char **ne_types = malloc(max_ne * sizeof(char *));
   char **ne_values = malloc(max_ne * sizeof(char *));
   int *mval = malloc(max_ne * sizeof(int));

Beautiful.

libssl/src/apps don't cast {m,re}alloc

[Thomas Pfaff]

Don't cast {m,re}alloc.  No point and it's inconsistent already.

Index: apps.c
===
RCS file: /cvs/src/lib/libssl/src/apps/apps.c,v
retrieving revision 1.42
diff -u -p -r1.42 apps.c
--- apps.c  22 Apr 2014 14:54:13 -  1.42
+++ apps.c  23 Apr 2014 07:20:29 -
@@ -216,7 +216,7 @@ chopup_args(ARGS * arg, char *buf, int *
i = 0;
if (arg-count == 0) {
arg-count = 20;
-   arg-data = (char **)malloc(sizeof(char *) * arg-count);
+   arg-data = malloc(sizeof(char *) * arg-count);
}
for (i = 0; i  arg-count; i++)
arg-data[i] = NULL;
@@ -236,8 +236,7 @@ chopup_args(ARGS * arg, char *buf, int *
if (num = arg-count) {
char **tmp_p;
int tlen = arg-count + 20;
-   tmp_p = (char **) realloc(arg-data,
-   sizeof(char *) * tlen);
+   tmp_p = realloc(arg-data, sizeof(char *) * tlen);
if (tmp_p == NULL)
return 0;
arg-data = tmp_p;
@@ -417,7 +416,7 @@ password_callback(char *buf, int bufsiz,
ok = UI_add_input_string(ui, prompt, ui_flags, buf,
PW_MIN_LENGTH, bufsiz - 1);
if (ok = 0  verify) {
-   buff = (char *) malloc(bufsiz);
+   buff = malloc(bufsiz);
ok = UI_add_verify_string(ui, prompt, ui_flags, buff,
PW_MIN_LENGTH, bufsiz - 1, buf);
}
Index: ca.c
===
RCS file: /cvs/src/lib/libssl/src/apps/ca.c,v
retrieving revision 1.46
diff -u -p -r1.46 ca.c
--- ca.c22 Apr 2014 13:32:17 -  1.46
+++ ca.c23 Apr 2014 07:20:29 -
@@ -1981,17 +1981,17 @@ again2:
goto err;
 
/* We now just add it to the database */
-   row[DB_type] = (char *) malloc(2);
+   row[DB_type] = malloc(2);
 
tm = X509_get_notAfter(ret);
-   row[DB_exp_date] = (char *) malloc(tm-length + 1);
+   row[DB_exp_date] = malloc(tm-length + 1);
memcpy(row[DB_exp_date], tm-data, tm-length);
row[DB_exp_date][tm-length] = '\0';
 
row[DB_rev_date] = NULL;
 
/* row[DB_serial] done already */
-   row[DB_file] = (char *) malloc(8);
+   row[DB_file] = malloc(8);
row[DB_name] = X509_NAME_oneline(X509_get_subject_name(ret), NULL, 0);
 
if ((row[DB_type] == NULL) || (row[DB_exp_date] == NULL) ||
@@ -2003,8 +2003,7 @@ again2:
row[DB_type][0] = 'V';
row[DB_type][1] = '\0';
 
-   if ((irow = (char **)malloc(sizeof(char *) * (DB_NUMBER + 1))) ==
-   NULL) {
+   if ((irow = malloc(sizeof(char *) * (DB_NUMBER + 1))) == NULL) {
BIO_printf(bio_err, Memory allocation failure\n);
goto err;
}
@@ -2245,17 +2244,17 @@ do_revoke(X509 * x509, CA_DB * db, int t
row[DB_serial], row[DB_name]);
 
/* We now just add it to the database */
-   row[DB_type] = (char *) malloc(2);
+   row[DB_type] = malloc(2);
 
tm = X509_get_notAfter(x509);
-   row[DB_exp_date] = (char *) malloc(tm-length + 1);
+   row[DB_exp_date] = malloc(tm-length + 1);
memcpy(row[DB_exp_date], tm-data, tm-length);
row[DB_exp_date][tm-length] = '\0';
 
row[DB_rev_date] = NULL;
 
/* row[DB_serial] done already */
-   row[DB_file] = (char *) malloc(8);
+   row[DB_file] = malloc(8);
 
/* row[DB_name] done already */
 
@@ -2268,7 +2267,7 @@ do_revoke(X509 * x509, CA_DB * db, int t
row[DB_type][0] = 'V';
row[DB_type][1] = '\0';
 
-   if ((irow = (char **)malloc(sizeof(char *) *
+   if ((irow = malloc(sizeof(char *) *
(DB_NUMBER + 1))) == NULL) {
BIO_printf(bio_err, Memory allocation failure\n);
goto err;
@@ -2405,7 +2404,7 @@ do_updatedb(CA_DB * db)
 
/* get actual time and make a string */
a_tm = X509_gmtime_adj(a_tm, 0);
-   a_tm_s = (char *) malloc(a_tm-length + 1);
+   a_tm_s = malloc(a_tm-length + 1);
if (a_tm_s == NULL) {
cnt = -1;
goto err;
Index: dgst.c
===
RCS file: /cvs/src/lib/libssl/src/apps/dgst.c,v
retrieving revision 1.27
diff -u -p -r1.27 dgst.c
--- dgst.c  18 Apr 2014 19:54:57 -  1.27
+++ dgst.c  23 Apr 2014 07:20:30 -
@@ -132,7 +132,7 @@ dgst_main(int argc, char **argv)
 
apps_startup();
 
-   if ((buf = (unsigned char *) malloc(BUFSIZE)) == 

Re: libssl/src/apps don't cast {m,re}alloc

[Jean-Philippe Ouellet]

This doesn't fix the problems, only removes markers alerting us to audit it.

Memory management in these files is still missing integer overflow checks,
NULL return checks, and is full of crazy abominations like:

X509_NAME *
parse_name(char *subject, long chtype, int multirdn)
{
size_t buflen = strlen(subject) + 1;/* ...
char *buf = malloc(buflen);
size_t max_ne = buflen / 2 + 1; /* maximum number of name elements */
char **ne_types = malloc(max_ne * sizeof(char *));
char **ne_values = malloc(max_ne * sizeof(char *));
int *mval = malloc(max_ne * sizeof(int));

I'm working on a more complete patch which addresses these issues too.