From 8803cf1c6547b1658f7210c1227fb9972b257f3b Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Date: Mar 15 2017 13:56:21 +0000 Subject: Fix some issues found with static analyzers --- diff --git a/freeradius-Fix-some-issues-found-with-static-analyzers.patch b/freeradius-Fix-some-issues-found-with-static-analyzers.patch new file mode 100644 index 0000000..759d9d3 --- /dev/null +++ b/freeradius-Fix-some-issues-found-with-static-analyzers.patch @@ -0,0 +1,262 @@ +From 7024d6ce061d57af65fe3a068803212581552f96 Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +Date: Fri, 10 Mar 2017 09:11:03 -0500 +Subject: [PATCH] Fix some issues found with static analyzers + +Fix some issues found with static analyzers. Includes the following. + +Coverity. Closes #1937 + +(cherry picked from commit 521e2a9bd3b1b49555bcd9fb90b03c456f616070) + +Allo session resumption for RadSec connectins. Closes #1936 + +(cherry picked from commit 43efa4321d7cd9fca1184f999e1cadeff3afda02) + +request->packet cannot be NULL. Helps with #1935 + +(cherry picked from commit 7f22c30476be495438d5bc4dbec2f618f09c0b15) + +remove unused variable + +(cherry picked from commit d9bfc70efbf575258425d2ca86160490e0c36a45) + +close open FDs on error, and use error path in more situations + +(cherry picked from commit e51af914bc5fdf879f821e6a1ecfe700bff937ca) + +return RLM_MODULE_FAIL for default switch statement + +(cherry picked from commit cdfa6e15065a4a616c96af516936117124a1c293) + +Remove always-false condition in rlm_eap_fast + +(cherry picked from commit 96d7a5e2bb393b4fd1b6cb6e0a6858e6c18eb96a) + +Remove always-false condition from cf_item_parse + +(cherry picked from commit 92624adf8170fb133b330fe02d8940a8bac86189) + +Ensure that error is always initialized + +(cherry picked from commit c483d8456e44747621334b318483c3a33752aaac) +--- + src/main/command.c | 15 ++++++++------- + src/main/conffile.c | 2 -- + src/main/process.c | 5 +++-- + src/main/tls.c | 12 ++++++------ + src/main/xlat.c | 6 +++++- + src/modules/rlm_cache/rlm_cache.c | 3 ++- + src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c | 3 --- + src/modules/rlm_mschap/rlm_mschap.c | 2 +- + 8 files changed, 25 insertions(+), 23 deletions(-) + +diff --git a/src/main/command.c b/src/main/command.c +index d3b729f9a..34c5268d7 100644 +--- a/src/main/command.c ++++ b/src/main/command.c +@@ -345,7 +345,7 @@ static int fr_server_domain_socket_perm(UNUSED char const *path, UNUSED uid_t ui + */ + static int fr_server_domain_socket_perm(char const *path, uid_t uid, gid_t gid) + { +- int dir_fd = -1, path_fd = -1, sock_fd = -1, parent_fd = -1; ++ int dir_fd = -1, sock_fd = -1, parent_fd = -1; + char const *name; + char *buff = NULL, *dir = NULL, *p; + +@@ -392,8 +392,9 @@ static int fr_server_domain_socket_perm(char const *path, uid_t uid, gid_t gid) + fr_strerror_printf("Failed determining parent directory"); + error: + talloc_free(dir); +- close(dir_fd); +- close(path_fd); ++ if (sock_fd >= 0) close(sock_fd); ++ if (dir_fd >= 0) close(dir_fd); ++ if (parent_fd >= 0) close(parent_fd); + return -1; + } + +@@ -459,7 +460,7 @@ static int fr_server_domain_socket_perm(char const *path, uid_t uid, gid_t gid) + if (ret < 0) { + fr_strerror_printf("Failed changing ownership of control socket directory: %s", + fr_syserror(errno)); +- return -1; ++ goto error; + } + /* + * Control socket dir already exists, but we still need to +@@ -527,7 +528,7 @@ static int fr_server_domain_socket_perm(char const *path, uid_t uid, gid_t gid) + if (client_fd >= 0) { + fr_strerror_printf("Control socket '%s' is already in use", path); + close(client_fd); +- return -1; ++ goto error; + } + } + +@@ -676,8 +677,8 @@ static int fr_server_domain_socket_perm(char const *path, uid_t uid, gid_t gid) + if (uid != (uid_t)-1) rad_seuid(euid); + if (gid != (gid_t)-1) rad_segid(egid); + +- close(dir_fd); +- close(path_fd); ++ if (dir_fd >= 0) close(dir_fd); ++ if (parent_fd >= 0) close(parent_fd); + + return sock_fd; + } +diff --git a/src/main/conffile.c b/src/main/conffile.c +index df78184bd..10c029a0e 100644 +--- a/src/main/conffile.c ++++ b/src/main/conffile.c +@@ -1474,7 +1474,6 @@ int cf_item_parse(CONF_SECTION *cs, char const *name, unsigned int type, void *d + + if (!value) { + if (required) { +- is_required: + cf_log_err(c_item, "Configuration item \"%s\" must have a value", name); + + return -1; +@@ -1620,7 +1619,6 @@ int cf_item_parse(CONF_SECTION *cs, char const *name, unsigned int type, void *d + } + } + +- if (required && !value) goto is_required; + if (cant_be_empty && (value[0] == '\0')) goto cant_be_empty; + + if (attribute) { +diff --git a/src/main/process.c b/src/main/process.c +index c5a690672..c3856c7a1 100644 +--- a/src/main/process.c ++++ b/src/main/process.c +@@ -2122,8 +2122,9 @@ static void remove_from_proxy_hash_nl(REQUEST *request, bool yank) + } + + #ifdef WITH_TCP +- rad_assert(request->proxy_listener != NULL); +- request->proxy_listener->count--; ++ if (request->proxy_listener) { ++ request->proxy_listener->count--; ++ } + #endif + request->proxy_listener = NULL; + +diff --git a/src/main/tls.c b/src/main/tls.c +index caa7e62ed..a72be2b63 100644 +--- a/src/main/tls.c ++++ b/src/main/tls.c +@@ -1360,7 +1360,7 @@ static int cbtls_new_session(SSL *ssl, SSL_SESSION *sess) + blob_len = i2d_SSL_SESSION(sess, NULL); + if (blob_len < 1) { + /* something went wrong */ +- RWDEBUG("Session serialisation failed, couldn't determine required buffer length"); ++ if (request) RWDEBUG("Session serialisation failed, couldn't determine required buffer length"); + return 0; + } + +@@ -1375,7 +1375,7 @@ static int cbtls_new_session(SSL *ssl, SSL_SESSION *sess) + p = sess_blob; + rv = i2d_SSL_SESSION(sess, &p); + if (rv != blob_len) { +- RWDEBUG("Session serialisation failed"); ++ if (request) RWDEBUG("Session serialisation failed"); + goto error; + } + +@@ -1384,8 +1384,8 @@ static int cbtls_new_session(SSL *ssl, SSL_SESSION *sess) + conf->session_cache_path, FR_DIR_SEP, buffer); + fd = open(filename, O_RDWR|O_CREAT|O_EXCL, 0600); + if (fd < 0) { +- RERROR("Session serialisation failed, failed opening session file %s: %s", +- filename, fr_syserror(errno)); ++ if (request) RERROR("Session serialisation failed, failed opening session file %s: %s", ++ filename, fr_syserror(errno)); + goto error; + } + +@@ -1409,7 +1409,7 @@ static int cbtls_new_session(SSL *ssl, SSL_SESSION *sess) + while (todo > 0) { + rv = write(fd, p, todo); + if (rv < 1) { +- RWDEBUG("Failed writing session: %s", fr_syserror(errno)); ++ if (request) RWDEBUG("Failed writing session: %s", fr_syserror(errno)); + close(fd); + goto error; + } +@@ -1417,7 +1417,7 @@ static int cbtls_new_session(SSL *ssl, SSL_SESSION *sess) + todo -= rv; + } + close(fd); +- RWDEBUG("Wrote session %s to %s (%d bytes)", buffer, filename, blob_len); ++ if (request) RWDEBUG("Wrote session %s to %s (%d bytes)", buffer, filename, blob_len); + } + + error: +diff --git a/src/main/xlat.c b/src/main/xlat.c +index 31987289c..aeac3a4c3 100644 +--- a/src/main/xlat.c ++++ b/src/main/xlat.c +@@ -1787,7 +1787,10 @@ static ssize_t xlat_tokenize_request(REQUEST *request, char const *fmt, xlat_exp + * much faster. + */ + tokens = talloc_typed_strdup(request, fmt); +- if (!tokens) return -1; ++ if (!tokens) { ++ error = "Out of memory"; ++ return -1; ++ } + + slen = xlat_tokenize_literal(request, tokens, head, false, &error); + +@@ -1806,6 +1809,7 @@ static ssize_t xlat_tokenize_request(REQUEST *request, char const *fmt, xlat_exp + */ + if (slen < 0) { + talloc_free(tokens); ++ + rad_assert(error != NULL); + + REMARKER(fmt, -slen, error); +diff --git a/src/modules/rlm_cache/rlm_cache.c b/src/modules/rlm_cache/rlm_cache.c +index 248de8bf9..54449747f 100644 +--- a/src/modules/rlm_cache/rlm_cache.c ++++ b/src/modules/rlm_cache/rlm_cache.c +@@ -126,7 +126,8 @@ static void CC_HINT(nonnull) cache_merge(rlm_cache_t *inst, REQUEST *request, rl + + RDEBUG2("Merging cache entry into request"); + +- if (c->packet && request->packet) { ++ if (c->packet) { ++ rad_assert(request->packet != NULL); + rdebug_pair_list(L_DBG_LVL_2, request, c->packet, "&request:"); + radius_pairmove(request, &request->packet->vps, fr_pair_list_copy(request->packet, c->packet), false); + } +diff --git a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c +index dba2c1462..95e521718 100644 +--- a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c ++++ b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c +@@ -1235,9 +1235,6 @@ PW_CODE eap_fast_process(eap_handler_t *eap_session, tls_session_t *tls_session) + + eap_fast_append_result(tls_session, code); + +- if (code == PW_CODE_ACCESS_REJECT) +- break; +- + if (t->pac.send) { + RDEBUG("Peer requires new PAC"); + eap_fast_send_pac_tunnel(request, tls_session); +diff --git a/src/modules/rlm_mschap/rlm_mschap.c b/src/modules/rlm_mschap/rlm_mschap.c +index aba15f826..c702f1b45 100644 +--- a/src/modules/rlm_mschap/rlm_mschap.c ++++ b/src/modules/rlm_mschap/rlm_mschap.c +@@ -1471,7 +1471,7 @@ static rlm_rcode_t mschap_error(rlm_mschap_t *inst, REQUEST *request, unsigned c + break; + + default: +- rad_assert(0); ++ return RLM_MODULE_FAIL; + } + mschap_add_reply(request, ident, "MS-CHAP-Error", buffer, strlen(buffer)); + +-- +2.11.0 + diff --git a/freeradius.spec b/freeradius.spec index d1fe6e3..02a420e 100644 --- a/freeradius.spec +++ b/freeradius.spec @@ -24,6 +24,7 @@ Source104: freeradius-tmpfiles.conf Patch1: freeradius-redhat-config.patch Patch2: freeradius-Use-system-crypto-policy-by-default.patch Patch3: freeradius-Relax-OpenSSL-permissions-for-default-key-files.patch +Patch4: freeradius-Fix-some-issues-found-with-static-analyzers.patch %global docdir %{?_pkgdocdir}%{!?_pkgdocdir:%{_docdir}/%{name}-%{version}} @@ -194,6 +195,7 @@ This plugin provides the REST support for the FreeRADIUS server project. %patch1 -p1 %patch2 -p1 %patch3 -p1 +%patch4 -p1 %build # Force compile/link options, extra security for network facing daemon @@ -800,6 +802,7 @@ exit 0 - Require OpenSSL version we built with, or newer, to avoid startup failures due to runtime OpenSSL version checks. Resolves: Bug#1299388 +- Fix some issues found with static analyzers. * Tue Mar 07 2017 Nikolai Kondrashov - 3.0.13-1 - Upgrade to upstream v3.0.13 release.