From 08488c86321f6fcb1da015ebfcc2b6fe3a2832d4 Mon Sep 17 00:00:00 2001 From: Phil Pennock Date: Fri, 18 May 2012 18:22:30 -0400 Subject: [PATCH] Fix three issues highlighted by clang analyser. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 2c0646b54..08fd2efe5 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 d69c2e4fe..1abd65781 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 79e2e3827..890665483 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 e9c4df919..372800783 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. */ -- 2.25.1