From 6614c958fc4fdf0078e41658e17f78cfed8549d7 Mon Sep 17 00:00:00 2001
From: Richard Russon <rich@flatcap.org>
Date: Mon, 13 Nov 2017 12:58:07 +0000
Subject: [PATCH] check retval of mutt_account_get{user,pass,login}()

Functions `mutt_account_getuser()`, `mutt_account_getpass()` and
`mutt_account_getlogin()` all return int, so check accordingly.

Also, make sure we log if any calls fail.
---
 conn/sasl.c       |  6 +++---
 conn/ssl.c        |  7 ++++---
 conn/ssl_gnutls.c |  3 ++-
 imap/auth_anon.c  |  2 +-
 imap/auth_cram.c  |  4 ++--
 imap/auth_gss.c   |  2 +-
 imap/auth_login.c |  4 ++--
 imap/auth_plain.c |  4 ++--
 imap/auth_sasl.c  |  2 +-
 mutt_account.c    | 49 ++++++++++++++++++++++++++++++++++++++---------
 mutt_account.h    |  8 ++++----
 nntp.c            |  4 ++--
 pop_auth.c        |  4 ++--
 smtp.c            |  3 ++-
 14 files changed, 68 insertions(+), 34 deletions(-)

diff --git a/conn/sasl.c b/conn/sasl.c
index 2d161c764..eef0afdf1 100644
--- a/conn/sasl.c
+++ b/conn/sasl.c
@@ -244,13 +244,13 @@ static int mutt_sasl_cb_authname(void *context, int id, const char **result, uns
 
   if (id == SASL_CB_AUTHNAME)
   {
-    if (mutt_account_getlogin(account))
+    if (mutt_account_getlogin(account) < 0)
       return SASL_FAIL;
     *result = account->login;
   }
   else
   {
-    if (mutt_account_getuser(account))
+    if (mutt_account_getuser(account) < 0)
       return SASL_FAIL;
     *result = account->user;
   }
@@ -280,7 +280,7 @@ static int mutt_sasl_cb_pass(sasl_conn_t *conn, void *context, int id, sasl_secr
   mutt_debug(2, "mutt_sasl_cb_pass: getting password for %s@%s:%u\n",
              account->login, account->host, account->port);
 
-  if (mutt_account_getpass(account))
+  if (mutt_account_getpass(account) < 0)
     return SASL_FAIL;
 
   len = strlen(account->pass);
diff --git a/conn/ssl.c b/conn/ssl.c
index 2a1bbc625..cbeacd493 100644
--- a/conn/ssl.c
+++ b/conn/ssl.c
@@ -360,13 +360,13 @@ static int ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata)
 {
   struct Account *account = (struct Account *) userdata;
 
-  if (mutt_account_getuser(account))
+  if (mutt_account_getuser(account) < 0)
     return 0;
 
   mutt_debug(2, "ssl_passwd_cb: getting password for %s@%s:%u\n", account->user,
              account->host, account->port);
 
-  if (mutt_account_getpass(account))
+  if (mutt_account_getpass(account) < 0)
     return 0;
 
   return snprintf(buf, size, "%s", account->pass);
@@ -710,7 +710,8 @@ static void ssl_get_client_cert(struct SslSockData *ssldata, struct Connection *
     SSL_CTX_use_PrivateKey_file(ssldata->ctx, SslClientCert, SSL_FILETYPE_PEM);
 
     /* if we are using a client cert, SASL may expect an external auth name */
-    mutt_account_getuser(&conn->account);
+    if (mutt_account_getuser(&conn->account) < 0)
+      mutt_debug(1, "Couldn't get user info\n");
   }
 }
 
diff --git a/conn/ssl_gnutls.c b/conn/ssl_gnutls.c
index a5f2ce831..872de9949 100644
--- a/conn/ssl_gnutls.c
+++ b/conn/ssl_gnutls.c
@@ -1013,7 +1013,8 @@ static void tls_get_client_cert(struct Connection *conn)
     *cnend = '\0';
 
   /* if we are using a client cert, SASL may expect an external auth name */
-  mutt_account_getuser(&conn->account);
+  if (mutt_account_getuser(&conn->account) < 0)
+    mutt_debug(1, "Couldn't get user info\n");
 
 err_dn:
   FREE(&dn);
diff --git a/imap/auth_anon.c b/imap/auth_anon.c
index de33b7f66..af67e46d3 100644
--- a/imap/auth_anon.c
+++ b/imap/auth_anon.c
@@ -56,7 +56,7 @@ enum ImapAuthRes imap_auth_anon(struct ImapData *idata, const char *method)
   if (!mutt_bit_isset(idata->capabilities, AUTH_ANON))
     return IMAP_AUTH_UNAVAIL;
 
-  if (mutt_account_getuser(&idata->conn->account))
+  if (mutt_account_getuser(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
 
   if (idata->conn->account.user[0] != '\0')
diff --git a/imap/auth_cram.c b/imap/auth_cram.c
index 8dc3af5c0..14f5c8e52 100644
--- a/imap/auth_cram.c
+++ b/imap/auth_cram.c
@@ -117,9 +117,9 @@ enum ImapAuthRes imap_auth_cram_md5(struct ImapData *idata, const char *method)
   mutt_message(_("Authenticating (CRAM-MD5)..."));
 
   /* get auth info */
-  if (mutt_account_getlogin(&idata->conn->account))
+  if (mutt_account_getlogin(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
-  if (mutt_account_getpass(&idata->conn->account))
+  if (mutt_account_getpass(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
 
   imap_cmd_start(idata, "AUTHENTICATE CRAM-MD5");
diff --git a/imap/auth_gss.c b/imap/auth_gss.c
index 2b1fff683..bac5629a9 100644
--- a/imap/auth_gss.c
+++ b/imap/auth_gss.c
@@ -118,7 +118,7 @@ enum ImapAuthRes imap_auth_gss(struct ImapData *idata, const char *method)
   if (!mutt_bit_isset(idata->capabilities, AGSSAPI))
     return IMAP_AUTH_UNAVAIL;
 
-  if (mutt_account_getuser(&idata->conn->account))
+  if (mutt_account_getuser(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
 
   /* get an IMAP service ticket for the server */
diff --git a/imap/auth_login.c b/imap/auth_login.c
index 952e3a0b2..65cadab5a 100644
--- a/imap/auth_login.c
+++ b/imap/auth_login.c
@@ -61,9 +61,9 @@ enum ImapAuthRes imap_auth_login(struct ImapData *idata, const char *method)
     return IMAP_AUTH_UNAVAIL;
   }
 
-  if (mutt_account_getuser(&idata->conn->account))
+  if (mutt_account_getuser(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
-  if (mutt_account_getpass(&idata->conn->account))
+  if (mutt_account_getpass(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
 
   mutt_message(_("Logging in..."));
diff --git a/imap/auth_plain.c b/imap/auth_plain.c
index 60a378840..a56e54337 100644
--- a/imap/auth_plain.c
+++ b/imap/auth_plain.c
@@ -53,9 +53,9 @@ enum ImapAuthRes imap_auth_plain(struct ImapData *idata, const char *method)
   enum ImapAuthRes res = IMAP_AUTH_SUCCESS;
   char buf[STRING];
 
-  if (mutt_account_getuser(&idata->conn->account))
+  if (mutt_account_getuser(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
-  if (mutt_account_getpass(&idata->conn->account))
+  if (mutt_account_getpass(&idata->conn->account) < 0)
     return IMAP_AUTH_FAILURE;
 
   mutt_message(_("Logging in..."));
diff --git a/imap/auth_sasl.c b/imap/auth_sasl.c
index 84e02ca63..ee76d73b7 100644
--- a/imap/auth_sasl.c
+++ b/imap/auth_sasl.c
@@ -82,7 +82,7 @@ enum ImapAuthRes imap_auth_sasl(struct ImapData *idata, const char *method)
      * 2. attempt sasl_client_start with only "AUTH=ANONYMOUS" capability
      * 3. if sasl_client_start fails, fall through... */
 
-    if (mutt_account_getuser(&idata->conn->account))
+    if (mutt_account_getuser(&idata->conn->account) < 0)
       return IMAP_AUTH_FAILURE;
 
     if (mutt_bit_isset(idata->capabilities, AUTH_ANON) &&
diff --git a/mutt_account.c b/mutt_account.c
index db532b3eb..f2a0d1c9b 100644
--- a/mutt_account.c
+++ b/mutt_account.c
@@ -32,7 +32,10 @@
 #include "url.h"
 
 /**
- * mutt_account_match - compare account info (host/port/user)
+ * mutt_account_match - Compare account info (host/port/user)
+ * @param a1 First Account
+ * @param a2 Second Account
+ * @retval 0 Accounts match
  */
 int mutt_account_match(const struct Account *a1, const struct Account *a2)
 {
@@ -78,7 +81,11 @@ int mutt_account_match(const struct Account *a1, const struct Account *a2)
 }
 
 /**
- * mutt_account_fromurl - fill account with information from url
+ * mutt_account_fromurl - Fill Account with information from url
+ * @param account Account to fill
+ * @param url     Url to parse
+ * @retval  0 Success
+ * @retval -1 Error
  */
 int mutt_account_fromurl(struct Account *account, struct Url *url)
 {
@@ -108,7 +115,9 @@ int mutt_account_fromurl(struct Account *account, struct Url *url)
 }
 
 /**
- * mutt_account_tourl - fill URL with info from account
+ * mutt_account_tourl - Fill URL with info from account
+ * @param account Source Account
+ * @param url     Url to fill
  *
  * The URL information is a set of pointers into account - don't free or edit
  * account until you've finished with url (make a copy of account if you need
@@ -172,7 +181,10 @@ void mutt_account_tourl(struct Account *account, struct Url *url)
 }
 
 /**
- * mutt_account_getuser - retrieve username into Account, if necessary
+ * mutt_account_getuser - Retrieve username into Account, if necessary
+ * @param account Account to fill
+ * @retval  0 Success
+ * @retval -1 Failure
  */
 int mutt_account_getuser(struct Account *account)
 {
@@ -210,6 +222,12 @@ int mutt_account_getuser(struct Account *account)
   return 0;
 }
 
+/**
+ * mutt_account_getlogin - Retrieve login info into Account, if necessary
+ * @param account Account to fill
+ * @retval  0 Success
+ * @retval -1 Failure
+ */
 int mutt_account_getlogin(struct Account *account)
 {
   /* already set */
@@ -228,17 +246,26 @@ int mutt_account_getlogin(struct Account *account)
 
   if (!(account->flags & MUTT_ACCT_LOGIN))
   {
-    mutt_account_getuser(account);
-    strfcpy(account->login, account->user, sizeof(account->login));
+    if (mutt_account_getuser(account) == 0)
+    {
+      strfcpy(account->login, account->user, sizeof(account->login));
+      account->flags |= MUTT_ACCT_LOGIN;
+    }
+    else
+    {
+      mutt_debug(1, "Couldn't get user info\n");
+      return -1;
+    }
   }
 
-  account->flags |= MUTT_ACCT_LOGIN;
-
   return 0;
 }
 
 /**
- * mutt_account_getpass - fetch password into Account, if necessary
+ * mutt_account_getpass - Fetch password into Account, if necessary
+ * @param account Account to fill
+ * @retval  0 Success
+ * @retval -1 Failure
  */
 int mutt_account_getpass(struct Account *account)
 {
@@ -279,6 +306,10 @@ int mutt_account_getpass(struct Account *account)
   return 0;
 }
 
+/**
+ * mutt_account_unsetpass - Unset Account's password
+ * @param account Account to modify
+ */
 void mutt_account_unsetpass(struct Account *account)
 {
   account->flags &= ~MUTT_ACCT_PASS;
diff --git a/mutt_account.h b/mutt_account.h
index c3637138d..8bb64bfd6 100644
--- a/mutt_account.h
+++ b/mutt_account.h
@@ -42,11 +42,11 @@ enum AccountType
 };
 
 /* account flags */
-#define MUTT_ACCT_PORT (1 << 0)
-#define MUTT_ACCT_USER (1 << 1)
+#define MUTT_ACCT_PORT  (1 << 0)
+#define MUTT_ACCT_USER  (1 << 1)
 #define MUTT_ACCT_LOGIN (1 << 2)
-#define MUTT_ACCT_PASS (1 << 3)
-#define MUTT_ACCT_SSL (1 << 4)
+#define MUTT_ACCT_PASS  (1 << 3)
+#define MUTT_ACCT_SSL   (1 << 4)
 
 int mutt_account_match(const struct Account *a1, const struct Account *m2);
 int mutt_account_fromurl(struct Account *account, struct Url *url);
diff --git a/nntp.c b/nntp.c
index 25f204545..8648ec5f7 100644
--- a/nntp.c
+++ b/nntp.c
@@ -313,8 +313,8 @@ static int nntp_auth(struct NntpServer *nserv)
   while (true)
   {
     /* get login and password */
-    if (mutt_account_getuser(&conn->account) || !conn->account.user[0] ||
-        mutt_account_getpass(&conn->account) || !conn->account.pass[0])
+    if ((mutt_account_getuser(&conn->account) < 0) || (conn->account.user[0] == '\0') ||
+        (mutt_account_getpass(&conn->account) < 0) || (conn->account.pass[0] == '\0'))
       break;
 
     /* get list of authenticators */
diff --git a/pop_auth.c b/pop_auth.c
index c3e5bcf9f..42478d075 100644
--- a/pop_auth.c
+++ b/pop_auth.c
@@ -343,8 +343,8 @@ int pop_authenticate(struct PopData *pop_data)
   int attempts = 0;
   int ret = POP_A_UNAVAIL;
 
-  if (mutt_account_getuser(acct) || !acct->user[0] ||
-      mutt_account_getpass(acct) || !acct->pass[0])
+  if ((mutt_account_getuser(acct) < 0) || (acct->user[0] == '\0') ||
+      (mutt_account_getpass(acct) < 0) || (acct->pass[0] == '\0'))
     return -3;
 
   if (PopAuthenticators && *PopAuthenticators)
diff --git a/smtp.c b/smtp.c
index 7aa9afb00..d308b3940 100644
--- a/smtp.c
+++ b/smtp.c
@@ -535,7 +535,8 @@ static int smtp_auth_plain(struct Connection *conn)
     if (mutt_strncasecmp(method, "plain", 5) == 0)
     {
       /* Get username and password. Bail out of any cannot be retrieved. */
-      if (mutt_account_getuser(&conn->account) || mutt_account_getpass(&conn->account))
+      if ((mutt_account_getuser(&conn->account) < 0) ||
+          (mutt_account_getpass(&conn->account) < 0))
       {
         goto error;
       }
-- 
2.40.0