Fix issues found by Coverity Scan

This commit is contained in:
Ondřej Lysoněk 2018-05-10 09:59:12 +02:00
parent 4d37c01b05
commit f2b42b7996
5 changed files with 376 additions and 1 deletions

View File

@ -0,0 +1,195 @@
From 01b646d2af0ed885d01d31a6479898a3c423a630 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= <olysonek@redhat.com>
Date: Thu, 26 Apr 2018 10:00:19 +0200
Subject: [PATCH 1/4] Fix rDNS with IPv6
Previously IPv6 addresses were not translated to hostnames for PAM to use.
---
privops.c | 3 ++-
sysdeputil.c | 28 +++++++++++++++-------------
sysdeputil.h | 5 ++++-
sysutil.c | 35 +++++++++++++++++++++++++++++++++++
sysutil.h | 4 ++++
5 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/privops.c b/privops.c
index f27c5c4..e577a27 100644
--- a/privops.c
+++ b/privops.c
@@ -383,7 +383,8 @@ handle_local_login(struct vsf_session* p_sess,
struct mystr* p_user_str,
const struct mystr* p_pass_str)
{
- if (!vsf_sysdep_check_auth(p_user_str, p_pass_str, &p_sess->remote_ip_str))
+ if (!vsf_sysdep_check_auth(p_sess, p_user_str, p_pass_str,
+ &p_sess->remote_ip_str))
{
return kVSFLoginFail;
}
diff --git a/sysdeputil.c b/sysdeputil.c
index 2063c87..4fe56c2 100644
--- a/sysdeputil.c
+++ b/sysdeputil.c
@@ -16,10 +16,6 @@
#include "tunables.h"
#include "builddefs.h"
-/* For gethostbyaddr, inet_addr */
-#include <netdb.h>
-#include <arpa/inet.h>
-
/* For Linux, this adds nothing :-) */
#include "port/porting_junk.h"
@@ -242,13 +238,15 @@ void vsf_remove_uwtmp(void);
#ifndef VSF_SYSDEP_HAVE_PAM
int
-vsf_sysdep_check_auth(struct mystr* p_user_str,
+vsf_sysdep_check_auth(struct vsf_session* p_sess,
+ struct mystr* p_user_str,
const struct mystr* p_pass_str,
const struct mystr* p_remote_host)
{
const char* p_crypted;
const struct passwd* p_pwd = getpwnam(str_getbuf(p_user_str));
(void) p_remote_host;
+ (void) p_sess;
if (p_pwd == NULL)
{
return 0;
@@ -322,14 +320,14 @@ static int pam_conv_func(int nmsg, const struct pam_message** p_msg,
static void vsf_auth_shutdown(void);
int
-vsf_sysdep_check_auth(struct mystr* p_user_str,
+vsf_sysdep_check_auth(struct vsf_session* p_sess,
+ struct mystr* p_user_str,
const struct mystr* p_pass_str,
const struct mystr* p_remote_host)
{
int retval = -1;
#ifdef PAM_RHOST
- struct sockaddr_in sin;
- struct hostent *host;
+ struct mystr hostname = INIT_MYSTR;
#endif
pam_item_t item;
const char* pam_user_name = 0;
@@ -354,13 +352,17 @@ vsf_sysdep_check_auth(struct mystr* p_user_str,
return 0;
}
#ifdef PAM_RHOST
- if (tunable_reverse_lookup_enable) {
- sin.sin_addr.s_addr = inet_addr(str_getbuf(p_remote_host));
- host = gethostbyaddr((char*)&sin.sin_addr.s_addr,sizeof(struct in_addr),AF_INET);
- if (host != (struct hostent*)0)
- retval = pam_set_item(s_pamh, PAM_RHOST, host->h_name);
+ if (tunable_reverse_lookup_enable)
+ {
+ if (vsf_sysutil_get_hostname(p_sess->p_remote_addr, &hostname) == 0)
+ {
+ retval = pam_set_item(s_pamh, PAM_RHOST, str_getbuf(&hostname));
+ str_free(&hostname);
+ }
else
+ {
retval = pam_set_item(s_pamh, PAM_RHOST, str_getbuf(p_remote_host));
+ }
} else {
retval = pam_set_item(s_pamh, PAM_RHOST, str_getbuf(p_remote_host));
}
diff --git a/sysdeputil.h b/sysdeputil.h
index 3b6b30a..6f2aa0a 100644
--- a/sysdeputil.h
+++ b/sysdeputil.h
@@ -5,6 +5,8 @@
#include "filesize.h"
#endif
+#include "session.h"
+
/* VSF_SYSDEPUTIL_H:
* Support for highly system dependent features, and querying for support
* or lack thereof
@@ -15,7 +17,8 @@ struct mystr;
/* Authentication of local users */
/* Return 0 for fail, 1 for success */
-int vsf_sysdep_check_auth(struct mystr* p_user,
+int vsf_sysdep_check_auth(struct vsf_session* p_sess,
+ struct mystr* p_user,
const struct mystr* p_pass,
const struct mystr* p_remote_host);
diff --git a/sysutil.c b/sysutil.c
index e847650..b68583b 100644
--- a/sysutil.c
+++ b/sysutil.c
@@ -2356,6 +2356,41 @@ vsf_sysutil_dns_resolve(struct vsf_sysutil_sockaddr** p_sockptr,
}
}
+int
+vsf_sysutil_get_hostname(struct vsf_sysutil_sockaddr *p_addr,
+ struct mystr* p_str)
+{
+ struct sockaddr *sa;
+ socklen_t sa_len = 0;
+ char hostname[NI_MAXHOST];
+ int res;
+
+ sa = &p_addr->u.u_sockaddr;
+ if (sa->sa_family == AF_INET)
+ {
+ sa_len = sizeof(struct sockaddr_in);
+ }
+ else if (sa->sa_family == AF_INET6)
+ {
+ sa_len = sizeof(struct sockaddr_in6);
+ }
+ else
+ {
+ die("can only support ipv4 and ipv6 currently");
+ }
+ res = getnameinfo(sa, sa_len, hostname, sizeof(hostname), NULL, 0,
+ NI_NAMEREQD);
+ if (res == 0)
+ {
+ str_alloc_text(p_str, hostname);
+ return 0;
+ }
+ else
+ {
+ return -1;
+ }
+}
+
struct vsf_sysutil_user*
vsf_sysutil_getpwuid(const unsigned int uid)
{
diff --git a/sysutil.h b/sysutil.h
index 7a59f13..2df14ed 100644
--- a/sysutil.h
+++ b/sysutil.h
@@ -7,6 +7,8 @@
#include "filesize.h"
#endif
+#include "str.h"
+
/* Return value queries */
int vsf_sysutil_retval_is_error(int retval);
enum EVSFSysUtilError
@@ -266,6 +268,8 @@ int vsf_sysutil_connect_timeout(int fd,
unsigned int wait_seconds);
void vsf_sysutil_dns_resolve(struct vsf_sysutil_sockaddr** p_sockptr,
const char* p_name);
+int vsf_sysutil_get_hostname(struct vsf_sysutil_sockaddr *p_addr,
+ struct mystr* p_str);
/* Option setting on sockets */
void vsf_sysutil_activate_keepalive(int fd);
void vsf_sysutil_rcvtimeo(int fd);
--
2.14.3

View File

@ -0,0 +1,32 @@
From 315f9720db94af3319c9550feaf473b9cf09aeac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= <olysonek@redhat.com>
Date: Thu, 3 May 2018 13:20:28 +0200
Subject: [PATCH 2/4] Always do chdir("/") after chroot()
Always do chdir("/") after chroot() to be more sure we'll never get out
of it. This will not affect the working directory after calling
vsf_sysutil_chroot(), because in the current state vsftpd always calls
vsf_sysutil_chroot(".").
---
sysutil.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/sysutil.c b/sysutil.c
index b68583b..3014c05 100644
--- a/sysutil.c
+++ b/sysutil.c
@@ -2588,6 +2588,11 @@ vsf_sysutil_chroot(const char* p_root_path)
{
die("chroot");
}
+ retval = chdir("/");
+ if (retval != 0)
+ {
+ die("chdir");
+ }
}
unsigned int
--
2.14.3

View File

@ -0,0 +1,33 @@
From ca27e6e34d89fc247a164ed7330735644f97d7d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= <olysonek@redhat.com>
Date: Wed, 9 May 2018 20:15:29 +0200
Subject: [PATCH 3/4] vsf_sysutil_rcvtimeo: Check return value of setsockopt
---
sysutil.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/sysutil.c b/sysutil.c
index 3014c05..de5f876 100644
--- a/sysutil.c
+++ b/sysutil.c
@@ -684,10 +684,15 @@ void
vsf_sysutil_rcvtimeo(int fd)
{
struct timeval tv;
+ int retval;
tv.tv_sec = tunable_data_connection_timeout;
tv.tv_usec = 0;
- setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(struct timeval));
+ retval = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(struct timeval));
+ if (retval != 0)
+ {
+ die("setsockopt: rcvtimeo");
+ }
}
void
--
2.14.3

View File

@ -0,0 +1,108 @@
From c7ac05fdf2a7b53d901bfc3afeb9a61916aaaaf1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= <olysonek@redhat.com>
Date: Wed, 9 May 2018 20:26:37 +0200
Subject: [PATCH 4/4] vsf_sysutil_get_tz: Check the return value of syscalls
Check the return value of syscalls. There's always the possibility that
they'll fail. (Failure of close() is not handled though, apart from EINTR.
The file is open read-only so it shouldn't fail, and even if it does,
it's not tragic.)
We return NULL in case of syscall failure. One might be tempted to simply
call die() when any kind of error occurs when parsing the timezone data,
but I think it's more in line with the behaviour of tzset(3) not to do
anything drastic in such a case (tzset() will silently use UTC when
the value given in the TZ environment variable is invalid).
---
sysutil.c | 46 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/sysutil.c b/sysutil.c
index de5f876..fd07d99 100644
--- a/sysutil.c
+++ b/sysutil.c
@@ -2647,12 +2647,12 @@ error:
die("reopening standard file descriptors to /dev/null failed");
}
-char* vsf_sysutil_get_tz()
+char* vsf_sysutil_get_tz(void)
{
char *ret_tz = NULL;
char buff[BUFTZSIZ];
off_t s_pos, e_pos;
- size_t rcnt, rest;
+ ssize_t rcnt, rest;
int fd;
if ((fd = open(F_LOCALTIME, O_RDONLY)) > -1)
@@ -2663,8 +2663,12 @@ char* vsf_sysutil_get_tz()
return NULL;
}
s_pos = e_pos > BUFTZSIZ ? e_pos - BUFTZSIZ : 0;
- lseek(fd, s_pos, SEEK_SET);
- rcnt = read(fd, buff, BUFTZSIZ);
+ if (lseek(fd, s_pos, SEEK_SET) == -1 ||
+ (rcnt = vsf_sysutil_read(fd, buff, BUFTZSIZ)) == -1)
+ {
+ close(fd);
+ return NULL;
+ }
if (rcnt && buff[rcnt-1] == '\n')
{
@@ -2680,10 +2684,25 @@ char* vsf_sysutil_get_tz()
int len = e_pos - s_pos - offset;
if (len)
{
- lseek(fd, s_pos + offset, SEEK_SET);
+ if (lseek(fd, s_pos + offset, SEEK_SET) == -1)
+ {
+ close(fd);
+ return NULL;
+ }
ret_tz = calloc(1, len+4);
+ if (ret_tz == NULL)
+ {
+ close(fd);
+ return NULL;
+ }
memcpy(ret_tz, "TZ=", 3);
- rcnt = read(fd, ret_tz+3, len);
+ rcnt = vsf_sysutil_read(fd, ret_tz+3, len);
+ if (rcnt == -1)
+ {
+ free(ret_tz);
+ close(fd);
+ return NULL;
+ }
}
break;
}
@@ -2693,11 +2712,20 @@ char* vsf_sysutil_get_tz()
}
rest = s_pos > BUFTZSIZ ? s_pos - BUFTZSIZ : 0;
s_pos -= rest;
- lseek(fd, s_pos, SEEK_SET);
- rcnt = read(fd, buff, rest);
+ if (lseek(fd, s_pos, SEEK_SET) == -1)
+ {
+ close(fd);
+ return NULL;
+ }
+ rcnt = vsf_sysutil_read(fd, buff, rest);
+ if (rcnt == -1)
+ {
+ close(fd);
+ return NULL;
+ }
} while (rcnt > 0);
- close (fd);
+ (void) vsf_sysutil_close_errno(fd);
}
return ret_tz;
--
2.14.3

View File

@ -2,7 +2,7 @@
Name: vsftpd
Version: 3.0.3
Release: 22%{?dist}
Release: 23%{?dist}
Summary: Very Secure Ftp Daemon
Group: System Environment/Daemons
@ -80,6 +80,10 @@ Patch48: 0048-Fix-default-value-of-strict_ssl_read_eof-in-man-page.patch
Patch49: 0049-Add-new-filename-generation-algorithm-for-STOU-comma.patch
Patch50: 0050-Don-t-link-with-libnsl.patch
Patch51: 0001-Improve-documentation-of-better_stou-in-the-man-page.patch
Patch52: 0001-Fix-rDNS-with-IPv6.patch
Patch53: 0002-Always-do-chdir-after-chroot.patch
Patch54: 0003-vsf_sysutil_rcvtimeo-Check-return-value-of-setsockop.patch
Patch55: 0004-vsf_sysutil_get_tz-Check-the-return-value-of-syscall.patch
%description
vsftpd is a Very Secure FTP daemon. It was written completely from
@ -149,6 +153,9 @@ mkdir -p $RPM_BUILD_ROOT/%{_var}/ftp/pub
%{_var}/ftp
%changelog
* Thu May 10 2018 Ondřej Lysoněk <olysonek@redhat.com> - 3.0.3-23
- Fix issues found by Coverity Scan
* Fri Apr 27 2018 Ondřej Lysoněk <olysonek@redhat.com> - 3.0.3-22
- Fix filename expansion in vsftpd_conf_migrate.sh