[exim-cvs] Fix three issues highlighted by clang analyser.

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] Fix three issues highlighted by clang analyser.
Gitweb: http://git.exim.org/exim.git/commitdiff/08488c86321f6fcb1da015ebfcc2b6fe3a2832d4
Commit:     08488c86321f6fcb1da015ebfcc2b6fe3a2832d4
Parent:     b5b317d2e0427dff8eb9640c57f3a2627835b82e
Author:     Phil Pennock <pdp@???>
AuthorDate: Fri May 18 18:22:30 2012 -0400
Committer:  Phil Pennock <pdp@???>
CommitDate: Fri May 18 18:22:30 2012 -0400


    Fix three issues highlighted by clang analyser.


    Only crash-plausible issue would require the Cambridge-specific
    iplookup router and a misconfiguration.


    Report from Marcin Mirosław
---
 doc/doc-txt/ChangeLog      |    5 +++++
 src/src/auths/spa.c        |   16 ++++++++++++----
 src/src/malware.c          |    3 +++
 src/src/routers/iplookup.c |    4 +++-
 4 files changed, 23 insertions(+), 5 deletions(-)


diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 2c0646b..08fd2ef 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -122,6 +122,11 @@ PP/28 Fix DCC dcc_header content corruption (stack memory referenced,
       read-only, out of scope).
       Patch from Wolfgang Breyha, report from Stuart Northfield.


+PP/29 Fix three issues highlighted by clang analyser static analysis.
+      Only crash-plausible issue would require the Cambridge-specific
+      iplookup router and a misconfiguration.
+      Report from Marcin Mirosław.
+


Exim version 4.77
-----------------
diff --git a/src/src/auths/spa.c b/src/src/auths/spa.c
index d69c2e4..1abd657 100644
--- a/src/src/auths/spa.c
+++ b/src/src/auths/spa.c
@@ -202,6 +202,11 @@ auth_vars[0] = expand_nstring[1] = msgbuf;
expand_nlength[1] = Ustrlen(msgbuf);
expand_nmax = 1;

+/* clean up globals which aren't referenced, but still shouldn't be left
+pointing to stack memory */
+#define CLEANUP_RETURN(Code) do { auth_vars[0] = expand_nstring[1] = NULL; \
+  expand_nlength[1] = expand_nmax = 0; return (Code); } while (0);
+
 debug_print_string(ablock->server_debug_string);    /* customized debug */


 /* look up password */
@@ -213,13 +218,13 @@ if (clearpass == NULL)
     {
     DEBUG(D_auth) debug_printf("auth_spa_server(): forced failure while "
       "expanding spa_serverpassword\n");
-    return FAIL;
+    CLEANUP_RETURN(FAIL);
     }
   else
     {
     DEBUG(D_auth) debug_printf("auth_spa_server(): error while expanding "
       "spa_serverpassword: %s\n", expand_string_message);
-    return DEFER;
+    CLEANUP_RETURN(DEFER);
     }
   }


@@ -234,11 +239,14 @@ if (memcmp(ntRespData,
       ((unsigned char*)responseptr)+IVAL(&responseptr->ntResponse.offset,0),
       24) == 0)
   /* success. we have a winner. */
+  {
+  int rc = auth_check_serv_cond(ablock);
+  CLEANUP_RETURN(rc);
+  }


/* Expand server_condition as an authorization check (PH) */
- return auth_check_serv_cond(ablock);

-return FAIL;
+CLEANUP_RETURN(FAIL);
}


diff --git a/src/src/malware.c b/src/src/malware.c
index 79e2e38..8906654 100644
--- a/src/src/malware.c
+++ b/src/src/malware.c
@@ -131,6 +131,9 @@ malware_in_file(uschar *eml_filename) {
set, but if that changes, then it should apply to these tests too */
unspool_mbox();

+ /* silence static analysis tools */
+ message_id = NULL;
+
return ret;
}

diff --git a/src/src/routers/iplookup.c b/src/src/routers/iplookup.c
index e9c4df9..3728007 100644
--- a/src/src/routers/iplookup.c
+++ b/src/src/routers/iplookup.c
@@ -142,7 +142,7 @@ iplookup_router_entry(
   address_item **addr_succeed)    /* put old address here on success */
 {
 uschar *query = NULL;
-uschar reply[256];
+uschar *reply;
 uschar *hostname, *reroute, *domain, *listptr;
 uschar host_buffer[256];
 host_item *host = store_get(sizeof(host_item));
@@ -161,6 +161,8 @@ pw = pw;
 DEBUG(D_route) debug_printf("%s router called for %s: domain = %s\n",
   rblock->name, addr->address, addr->domain);


+reply = store_get(256);
+
/* Build the query string to send. If not explicitly given, a default of
"user@domain user@domain" is used. */