From 997ceda29bf9c8e54a10f5c0cec7297b067ad5e7 Mon Sep 17 00:00:00 2001 From: Eduardo Chappa Date: Thu, 6 Dec 2018 16:23:29 -0700 Subject: * Fix a crash in the S/MIME configuration screen when a user turned off S/MIME, and then reenabled it. This crash was due to a double free of memory. To avoid this in the future, we created a function free_x509_store(), which whenever called, will only free memory once. A similar crash would occur when one attempted to enter the S/MIME configuration screen if S/MIME was turned off. In this case, Alpine would try to dereference a null pointer. --- alpine/smime.c | 3 ++- pith/pine.hlp | 6 +++++- pith/smime.c | 7 +++---- pith/smkeys.c | 16 ++++++++++++---- pith/smkeys.h | 1 + 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/alpine/smime.c b/alpine/smime.c index 641ef264..dd73ba03 100644 --- a/alpine/smime.c +++ b/alpine/smime.c @@ -990,7 +990,8 @@ smime_config_init_display(struct pine *ps, CONF_S **ctmp, CONF_S **first_line) #endif /* APPLEKEYCHAIN */ - if(SMHOLDERTYPE(Private) == Keychain + if(ps_global->smime + && SMHOLDERTYPE(Private) == Keychain && SMHOLDERTYPE(Public) == Keychain && SMHOLDERTYPE(CACert) == Keychain) return; diff --git a/pith/pine.hlp b/pith/pine.hlp index 4f6ce153..3d794087 100644 --- a/pith/pine.hlp +++ b/pith/pine.hlp @@ -140,7 +140,7 @@ with help text for the config screen and the composer that didn't have any reasonable place to be called from. Dummy change to get revision in pine.hlp ============= h_revision ================= -Alpine Commit 301 2018-08-15 21:08:38 +Alpine Commit 311 2018-12-06 16:16:12 ============= h_news ================= @@ -237,6 +237,10 @@ Bugs that have been addressed include:
  • Crash when a CA certificate failed to load, and user attempted to view certificate information of other certificate authorities. +
  • Crash in the S/MIME configuration screen when a user turned off S/MIME, and + then reenabled it. Also crash when attempting to enter the S/MIME + configuration screen if S/MIME was turned off. +
  • Deactivate some color code from Pico (as standalone editor in the windows version) until I find a way to activate it again. This is not critical and it is not something that PC-Pico must have (some diff --git a/pith/smime.c b/pith/smime.c index 77376d5a..f64d81b2 100644 --- a/pith/smime.c +++ b/pith/smime.c @@ -1204,8 +1204,7 @@ renew_cert_data(CertList **data, WhichCerts ctype) else ps_global->smime->cacertlist = *data; } - X509_STORE_free(store); - store = NULL; + free_x509_store(&store); } } setup_certs_backup_by_type(ctype); @@ -1224,7 +1223,7 @@ smime_deinit(void) { dprint((9, "smime_deinit()")); app_RAND_write_file(NULL); - if (s_cert_store != NULL) X509_STORE_free(s_cert_store); + if (s_cert_store != NULL) free_x509_store(&s_cert_store); #ifdef ERR_free_strings ERR_free_strings(); #endif /* ERR_free_strings */ @@ -1240,7 +1239,7 @@ renew_store(void) { if(ps_global->smime->inited){ if(s_cert_store != NULL) - X509_STORE_free(s_cert_store); + free_x509_store(&s_cert_store); s_cert_store = get_ca_store(); } } diff --git a/pith/smkeys.c b/pith/smkeys.c index 5fd7f97c..04ae8c18 100644 --- a/pith/smkeys.c +++ b/pith/smkeys.c @@ -786,7 +786,7 @@ get_ca_store(void) if(!(lookup=X509_STORE_add_lookup(store, X509_LOOKUP_file()))){ dprint((9, "X509_STORE_add_lookup() failed")); - X509_STORE_free(store); + free_x509_store(&store); return NULL; } @@ -794,21 +794,21 @@ get_ca_store(void) && ps_global->smime->cacontent){ if(!mem_add_extra_cacerts(ps_global->smime->cacontent, lookup)){ - X509_STORE_free(store); + free_x509_store(&store); return NULL; } } else if(ps_global->smime && ps_global->smime->catype == Directory && ps_global->smime->capath){ if(add_certs_in_dir(lookup, ps_global->smime->capath, ".crt", &ps_global->smime->cacertlist) < 0){ - X509_STORE_free(store); + free_x509_store(&store); return NULL; } resort_certificates(&ps_global->smime->cacertlist, CACert); } if(!(lookup = X509_STORE_add_lookup(store, X509_LOOKUP_hash_dir()))){ - X509_STORE_free(store); + free_x509_store(&store); return NULL; } @@ -820,6 +820,14 @@ get_ca_store(void) return store; } +void +free_x509_store(X509_STORE **xstore) +{ + if(xstore == NULL || *xstore == NULL) + return; + X509_STORE_free(*xstore); + *xstore = NULL; +} EVP_PKEY * load_key(PERSONAL_CERT *pc, char *pass, int flag) diff --git a/pith/smkeys.h b/pith/smkeys.h index e37eea3b..b9c256f6 100644 --- a/pith/smkeys.h +++ b/pith/smkeys.h @@ -72,6 +72,7 @@ typedef struct personal_cert { /* exported protoypes */ int add_certs_in_dir(X509_LOOKUP *lookup, char *path, char *ext, CertList **cdata); X509_STORE *get_ca_store(void); +void free_x509_store(X509_STORE **); PERSONAL_CERT *get_personal_certs(char *d); X509 *get_cert_for(char *email, WhichCerts ctype, int tolower); void save_cert_for(char *email, X509 *cert, WhichCerts ctype); -- cgit v1.2.3-54-g00ecf