Fix memory leak in proxy connection flow

This commit is contained in:
Pavel Zhukov 2017-08-21 15:43:33 +02:00
parent 9353dccefb
commit 42d973c99f
2 changed files with 268 additions and 10 deletions

258
nmap-7.60-memleak.patch Normal file
View File

@ -0,0 +1,258 @@
commit 7e876de88995807350e401e277384a10fe5b9d74
Author: nnposter <nnposter@e0a8ed71-7df4-0310-8962-fdc924857419>
Date: Fri Aug 18 02:24:37 2017 +0000
Makes sure that nsock_pool is properly disposed of if the proxy connection fails. Closes #973
diff --git a/ncat/ncat_connect.c b/ncat/ncat_connect.c
index d8c73ab1b..5695800a3 100644
--- a/ncat/ncat_connect.c
+++ b/ncat/ncat_connect.c
@@ -1049,7 +1049,10 @@ int ncat_connect(void)
}
if (connect_socket == -1)
+ {
+ nsock_pool_delete(mypool);
return 1;
+ }
/* Clear out whatever is left in the socket buffer which may be
already sent by proxy server along with http response headers. */
//line = socket_buffer_remainder(&stateful_buf, &n);
commit 19d62a81013cbab56957b982f7a581dd7622a9d1
Author: Pavel Zhukov <pzhukov@redhat.com>
Date: Fri Aug 18 10:30:20 2017 +0200
Makes sure that proxy_auth is properly disposed of if the proxy connection fails.
In case of error in proxy connection initialization proxy_auth pointer
goes out of scope and memory leak reported by static analizer tool.
While this is not critical because of application exit the fix is trivial.
diff --git a/ncat/ncat_connect.c b/ncat/ncat_connect.c
index 5695800a3..bad673770 100644
--- a/ncat/ncat_connect.c
+++ b/ncat/ncat_connect.c
@@ -664,7 +664,7 @@ static int do_proxy_socks5(void)
int sd,len,lenfqdn;
struct socks5_request socks5msg2;
struct socks5_auth socks5auth;
- char *proxy_auth;
+ char *proxy_auth = NULL;
char *username;
char *password;
@@ -695,21 +695,18 @@ static int do_proxy_socks5(void)
if (send(sd, (char *) &socks5msg, len, 0) < 0) {
loguser("Error: proxy request: %s.\n", socket_strerror(socket_errno()));
- close(sd);
- return -1;
+ goto error;
}
/* first response just two bytes, version and auth method */
if (socket_buffer_readcount(&stateful_buf, socksbuf, 2) < 0) {
loguser("Error: malformed first response from proxy.\n");
- close(sd);
- return -1;
+ goto error;
}
if (socksbuf[0] != 5){
loguser("Error: got wrong server version in response.\n");
- close(sd);
- return -1;
+ goto error;
}
switch(socksbuf[1]) {
@@ -720,8 +717,7 @@ static int do_proxy_socks5(void)
case SOCKS5_AUTH_GSSAPI:
loguser("GSSAPI authentication method not supported.\n");
- close(sd);
- return -1;
+ goto error;
case SOCKS5_AUTH_USERPASS:
if (o.verbose)
@@ -729,14 +725,12 @@ static int do_proxy_socks5(void)
if(!o.proxy_auth){
loguser("Error: proxy requested to do authentication, but no credentials were provided.\n");
- close(sd);
- return -1;
+ goto error;
}
if (strlen(o.proxy_auth) > SOCKS_BUFF_SIZE-2){
loguser("Error: username and password are too long to fit into buffer.\n");
- close(sd);
- return -1;
+ goto error;
}
/* Split up the proxy auth argument. */
@@ -744,10 +738,8 @@ static int do_proxy_socks5(void)
username = strtok(proxy_auth, ":");
password = strtok(NULL, ":");
if (password == NULL || username == NULL) {
- free(proxy_auth);
loguser("Error: empty username or password.\n");
- close(sd);
- return -1;
+ goto error;
}
/*
@@ -776,28 +768,24 @@ static int do_proxy_socks5(void)
if (send(sd, (char *) &socks5auth, len, 0) < 0) {
loguser("Error: sending proxy authentication.\n");
- close(sd);
- return -1;
+ goto error;
}
if (socket_buffer_readcount(&stateful_buf, socksbuf, 2) < 0) {
loguser("Error: malformed proxy authentication response.\n");
- close(sd);
- return -1;
+ goto error;
}
if (socksbuf[0] != 1 || socksbuf[1] != 0) {
loguser("Error: authentication failed.\n");
- close(sd);
- return -1;
+ goto error;
}
break;
default:
loguser("Error - can't choose any authentication method.\n");
- close(sd);
- return -1;
+ goto error;
}
zmem(&socks5msg2,sizeof(socks5msg2));
@@ -826,8 +814,7 @@ static int do_proxy_socks5(void)
lenfqdn=strlen(o.target);
if (lenfqdn > SOCKS_BUFF_SIZE-5){
loguser("Error: host name too long.\n");
- close(sd);
- return -1;
+ goto error;
}
socks5msg2.dst[0]=lenfqdn;
memcpy(socks5msg2.dst+1,o.target,lenfqdn);
@@ -839,21 +826,18 @@ static int do_proxy_socks5(void)
if (len > sizeof(socks5msg2)){
loguser("Error: address information too large.\n");
- close(sd);
- return -1;
+ goto error;
}
if (send(sd, (char *) &socks5msg2, len, 0) < 0) {
loguser("Error: sending proxy request: %s.\n", socket_strerror(socket_errno()));
- close(sd);
- return -1;
+ goto error;
}
/* TODO just two bytes for now, need to read more for bind */
if (socket_buffer_readcount(&stateful_buf, socksbuf, 2) < 0) {
loguser("Error: malformed second response from proxy.\n");
- close(sd);
- return -1;
+ goto error;
}
switch(socksbuf[1]) {
@@ -863,43 +847,40 @@ static int do_proxy_socks5(void)
break;
case 1:
loguser("Error: general SOCKS server failure.\n");
- close(sd);
- return -1;
+ goto error;
case 2:
loguser("Error: connection not allowed by ruleset.\n");
- close(sd);
- return -1;
+ goto error;
case 3:
loguser("Error: Network unreachable.\n");
- close(sd);
- return -1;
+ goto error;
case 4:
loguser("Error: Host unreachable.\n");
- close(sd);
- return -1;
+ goto error;
case 5:
loguser("Error: Connection refused.\n");
- close(sd);
- return -1;
+ goto error;
case 6:
loguser("Error: TTL expired.\n");
- close(sd);
- return -1;
+ goto error;
case 7:
loguser("Error: Command not supported.\n");
- close(sd);
- return -1;
+ goto error;
case 8:
loguser("Error: Address type not supported.\n");
- close(sd);
- return -1;
+ goto error;
default:
loguser("Error: unassigned value in the reply.\n");
- close(sd);
- return -1;
+ goto error;
}
return(sd);
+error:
+ if (proxy_auth != NULL)
+ free(proxy_auth);
+ close(sd);
+ return -1;
+
}
static nsock_iod new_iod(nsock_pool mypool) {
commit e61c827fb0e7c0df8539b10ac31e67c82cf1425f
Author: Pavel Zhukov <pzhukov@redhat.com>
Date: Fri Aug 18 13:15:45 2017 +0200
Always free proxy_auth
diff --git a/ncat/ncat_connect.c b/ncat/ncat_connect.c
index bad673770..d77b6d1bc 100644
--- a/ncat/ncat_connect.c
+++ b/ncat/ncat_connect.c
@@ -874,10 +874,10 @@ static int do_proxy_socks5(void)
goto error;
}
+ free(proxy_auth);
return(sd);
error:
- if (proxy_auth != NULL)
- free(proxy_auth);
+ free(proxy_auth);
close(sd);
return -1;

View File

@ -8,7 +8,7 @@ Name: nmap
Epoch: 2 Epoch: 2
Version: 7.60 Version: 7.60
#global prerelease TEST5 #global prerelease TEST5
Release: 7%{?dist} Release: 8%{?dist}
# Uses combination of licenses based on GPL license, but with extra modification # Uses combination of licenses based on GPL license, but with extra modification
# so it got its own license tag rhbz#1055861 # so it got its own license tag rhbz#1055861
License: Nmap License: Nmap
@ -17,7 +17,6 @@ Requires: %{name}-ncat = %{epoch}:%{version}-%{release}
Source0: http://nmap.org/dist/%{name}-%{version}%{?prerelease}.tar.bz2 Source0: http://nmap.org/dist/%{name}-%{version}%{?prerelease}.tar.bz2
Source1: zenmap.desktop Source1: zenmap.desktop
Source2: zenmap-root.pamd Source2: zenmap-root.pamd
Source3: zenmap-root.consoleapps
#prevent possible race condition for shtool, rhbz#158996 #prevent possible race condition for shtool, rhbz#158996
Patch1: nmap-4.03-mktemp.patch Patch1: nmap-4.03-mktemp.patch
@ -30,6 +29,10 @@ Patch5: ncat_reg_stdin.diff
Patch6: nmap-6.25-displayerror.patch Patch6: nmap-6.25-displayerror.patch
## https://github.com/nmap/nmap/commit/fd0db097498d5ff29f647508a80915d4d0e8d84a ## https://github.com/nmap/nmap/commit/fd0db097498d5ff29f647508a80915d4d0e8d84a
Patch7: nmap-7.60-bundled_libssh2_libz.patch Patch7: nmap-7.60-bundled_libssh2_libz.patch
# https://github.com/nmap/nmap/pull/973
# https://github.com/nmap/nmap/pull/975
Patch8: nmap-7.60-memleak.patch
URL: http://nmap.org/ URL: http://nmap.org/
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
@ -97,6 +100,7 @@ BuildArch: noarch
%patch5 -p1 -b .ncat_reg_stdin %patch5 -p1 -b .ncat_reg_stdin
%patch6 -p1 -b .displayerror %patch6 -p1 -b .displayerror
%patch7 -p1 -b .libssh2 %patch7 -p1 -b .libssh2
%patch8 -p1 -b .memleak
#be sure we're not using tarballed copies of some libraries #be sure we're not using tarballed copies of some libraries
#rm -rf liblua libpcap libpcre macosx mswin32 ###TODO### #rm -rf liblua libpcap libpcre macosx mswin32 ###TODO###
@ -142,14 +146,9 @@ rmdir %{buildroot}%{_datadir}/ncat
#do not include uninstall script #do not include uninstall script
rm -f %{buildroot}%{_bindir}/uninstall_ndiff rm -f %{buildroot}%{_bindir}/uninstall_ndiff
#use consolehelper
rm -f %{buildroot}%{_datadir}/applications/zenmap*.desktop rm -f %{buildroot}%{_datadir}/applications/zenmap*.desktop
rm -f %{buildroot}%{_datadir}/zenmap/su-to-zenmap.sh mkdir -p %{buildroot}%{_sysconfdir}/pam.d
ln -s consolehelper %{buildroot}%{_bindir}/zenmap-root
mkdir -p %{buildroot}%{_sysconfdir}/pam.d \
%{buildroot}%{_sysconfdir}/security/console.apps
install -m 0644 %{SOURCE2} %{buildroot}%{_sysconfdir}/pam.d/zenmap-root install -m 0644 %{SOURCE2} %{buildroot}%{_sysconfdir}/pam.d/zenmap-root
install -m 0644 %{SOURCE3} %{buildroot}%{_sysconfdir}/security/console.apps/zenmap-root
cp docs/zenmap.1 %{buildroot}%{_mandir}/man1/ cp docs/zenmap.1 %{buildroot}%{_mandir}/man1/
gzip %{buildroot}%{_mandir}/man1/* || : gzip %{buildroot}%{_mandir}/man1/* || :
@ -225,8 +224,6 @@ rm -rf %{buildroot}
%files frontend -f zenmap.lang %files frontend -f zenmap.lang
%defattr(-,root,root) %defattr(-,root,root)
%config(noreplace) %{_sysconfdir}/pam.d/zenmap-root %config(noreplace) %{_sysconfdir}/pam.d/zenmap-root
%config(noreplace) %{_sysconfdir}/security/console.apps/zenmap-root
%{_bindir}/zenmap-root
%{_bindir}/zenmap %{_bindir}/zenmap
%{_bindir}/nmapfe %{_bindir}/nmapfe
%{_bindir}/xnmap %{_bindir}/xnmap
@ -240,6 +237,9 @@ rm -rf %{buildroot}
%{_mandir}/man1/xnmap.1.gz %{_mandir}/man1/xnmap.1.gz
%changelog %changelog
* Mon Aug 21 2017 Pavel Zhukov <pzhukov@redhat.com> - 2:7.60-8
- Fix memory leaks on error
* Thu Aug 3 2017 Pavel Zhukov <pzhukov@redhat.com> - 2:7.60-7 * Thu Aug 3 2017 Pavel Zhukov <pzhukov@redhat.com> - 2:7.60-7
- Use upstream patch - Use upstream patch