[PATCH] Remove sock_exec code
Gary Lockyer
gary at catalyst.net.nz
Thu Nov 16 18:18:07 UTC 2017
Updated patch attached, have revised the commit message in line with
Andrews comments
Gary
On 15/11/17 00:10, Andrew Bartlett via samba-technical wrote:
> I think you should mention that you are motivated by past exploits of
> Samba that used sock_exec as a proxy for system() matching a talloc
> destructor prototype.
>
> See for example
> https://gist.github.com/worawit/051e881fc94fe4a49295
>
> and referencing the Red Hat post at:
>
> https://access.redhat.com/blogs/766093/posts/1976553
>
> The same motivation is for the close-on-exec change, making a repeat of
> this exploit just a little more miserable by not allowing something
> called via system() access to the FD back to the client.
>
> Thanks,
>
> Andrew Bartlett
>
> On Thu, 2017-11-09 at 15:09 +1300, Gary Lockyer via samba-technical
> wrote:
>> Updated patch attached, removes the man page entry.
>>
>> On 09/11/17 05:11, Andreas Schneider via samba-technical wrote:
>>> On Sunday, 5 November 2017 18:54:32 CET Gary Lockyer via samba-technical
>>> wrote:
>>>> Patch to remove the sock_exec code, my understanding is that this was
>>>> originally test support code and is no longer used.
>>>>
>>>> Comments and reviews appreciated
>>>
>>> man smbclient -> LIBSMB_PROG
>>>
>>> I think you should remove that too if you want to get rid of it :-)
>>>
>>>
>>> andreas
>>>
-------------- next part --------------
From adc924cc00297faddf174b94308e883a1d35c8bb Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 3 Nov 2017 13:35:41 +1300
Subject: [PATCH] source3: remove sock_exec
Remove the sock_exec code which is no longer needed and additionally has been
used by exploit code.
This was originally test support code, the tests relying on the sock_exec
code have been removed.
Past exploits have used sock_exec as a proxy for system() matching a talloc
destructor prototype.
See for example:
Exploit for Samba vulnerabilty (CVE-2015-0240) at
https://gist.github.com/worawit/051e881fc94fe4a49295
and the Red Hat post at
https://access.redhat.com/blogs/766093/posts/1976553
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
docs-xml/manpages/smbclient.1.xml | 6 --
source3/include/proto.h | 4 --
source3/lib/sock_exec.c | 118 --------------------------------------
source3/libsmb/cliconnect.c | 14 -----
source3/wscript_build | 1 -
testsuite/build_farm/basicsmb.fns | 2 -
6 files changed, 145 deletions(-)
delete mode 100644 source3/lib/sock_exec.c
diff --git a/docs-xml/manpages/smbclient.1.xml b/docs-xml/manpages/smbclient.1.xml
index 0325982..1349a67 100644
--- a/docs-xml/manpages/smbclient.1.xml
+++ b/docs-xml/manpages/smbclient.1.xml
@@ -1216,12 +1216,6 @@
the password of the person using the client. This information is
used only if the protocol level is high enough to support
session-level passwords. </para>
-
- <para>The variable <envar>LIBSMB_PROG</envar> may contain
- the path, executed with system(), which the client should connect
- to instead of connecting to a server. This functionality is primarily
- intended as a development aid, and works best when using a LMHOSTS
- file</para>
</refsect1>
diff --git a/source3/include/proto.h b/source3/include/proto.h
index c86cd44..fa87407 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -158,10 +158,6 @@ int smbrun_no_sanitize(const char *cmd, int *outfd, char * const *env);
int smbrun(const char *cmd, int *outfd, char * const *env);
int smbrunsecret(const char *cmd, const char *secret);
-/* The following definitions come from lib/sock_exec.c */
-
-int sock_exec(const char *prog);
-
/* The following definitions come from lib/substitute.c */
void free_local_machine_name(void);
diff --git a/source3/lib/sock_exec.c b/source3/lib/sock_exec.c
deleted file mode 100644
index 3d2ace9..0000000
--- a/source3/lib/sock_exec.c
+++ /dev/null
@@ -1,118 +0,0 @@
-/*
- Unix SMB/CIFS implementation.
- Samba utility functions
- Copyright (C) Andrew Tridgell 1992-1998
- Copyright (C) Tim Potter 2000-2001
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-
- You should have received a copy of the GNU General Public License
- along with this program. If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include "includes.h"
-
-/*******************************************************************
-this is like socketpair but uses tcp. It is used by the Samba
-regression test code
-The function guarantees that nobody else can attach to the socket,
-or if they do that this function fails and the socket gets closed
-returns 0 on success, -1 on failure
-the resulting file descriptors are symmetrical
- ******************************************************************/
-static int socketpair_tcp(int fd[2])
-{
- int listener;
- struct sockaddr_in sock;
- struct sockaddr_in sock2;
- socklen_t socklen = sizeof(sock);
- int connect_done = 0;
-
- fd[0] = fd[1] = listener = -1;
-
- memset(&sock, 0, sizeof(sock));
-
- if ((listener = socket(PF_INET, SOCK_STREAM, 0)) == -1) goto failed;
-
- memset(&sock2, 0, sizeof(sock2));
-#ifdef HAVE_SOCK_SIN_LEN
- sock2.sin_len = sizeof(sock2);
-#endif
- sock2.sin_family = PF_INET;
-
- if (bind(listener, (struct sockaddr *)&sock2, sizeof(sock2)) != 0) goto failed;
-
- if (listen(listener, 1) != 0) goto failed;
-
- if (getsockname(listener, (struct sockaddr *)&sock, &socklen) != 0) goto failed;
-
- if ((fd[1] = socket(PF_INET, SOCK_STREAM, 0)) == -1) goto failed;
-
- set_blocking(fd[1], 0);
-
- sock.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
-
- if (connect(fd[1], (struct sockaddr *)&sock, socklen) == -1) {
- if (errno != EINPROGRESS) goto failed;
- } else {
- connect_done = 1;
- }
-
- if ((fd[0] = accept(listener, (struct sockaddr *)&sock, &socklen)) == -1) goto failed;
-
- if (connect_done == 0) {
- if (connect(fd[1], (struct sockaddr *)&sock, socklen) != 0
- && errno != EISCONN) goto failed;
- }
- close(listener);
-
- set_blocking(fd[1], 1);
-
- /* all OK! */
- return 0;
-
- failed:
- if (fd[0] != -1) close(fd[0]);
- if (fd[1] != -1) close(fd[1]);
- if (listener != -1) close(listener);
- return -1;
-}
-
-
-/*******************************************************************
-run a program on a local tcp socket, this is used to launch smbd
-when regression testing
-the return value is a socket which is attached to a subprocess
-running "prog". stdin and stdout are attached. stderr is left
-attached to the original stderr
- ******************************************************************/
-int sock_exec(const char *prog)
-{
- int fd[2];
- if (socketpair_tcp(fd) != 0) {
- DEBUG(0,("socketpair_tcp failed (%s)\n", strerror(errno)));
- return -1;
- }
- if (fork() == 0) {
- close(fd[0]);
- close(0);
- close(1);
- if (dup(fd[1]) == -1) {
- exit(1);
- }
- if (dup(fd[1]) == -1) {
- exit(1);
- }
- exit(system(prog));
- }
- close(fd[1]);
- return fd[0];
-}
diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index 70bcead..26bf569 100644
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -2505,7 +2505,6 @@ static struct tevent_req *cli_connect_sock_send(
{
struct tevent_req *req, *subreq;
struct cli_connect_sock_state *state;
- const char *prog;
struct sockaddr_storage *addrs;
unsigned i, num_addrs;
NTSTATUS status;
@@ -2516,19 +2515,6 @@ static struct tevent_req *cli_connect_sock_send(
return NULL;
}
- prog = getenv("LIBSMB_PROG");
- if (prog != NULL) {
- state->fd = sock_exec(prog);
- if (state->fd == -1) {
- status = map_nt_error_from_unix(errno);
- tevent_req_nterror(req, status);
- } else {
- state->port = 0;
- tevent_req_done(req);
- }
- return tevent_req_post(req, ev);
- }
-
if ((pss == NULL) || is_zero_addr(pss)) {
/*
diff --git a/source3/wscript_build b/source3/wscript_build
index dc7a51c..f4c3eb5 100644
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -304,7 +304,6 @@ bld.SAMBA3_SUBSYSTEM('samba3util',
lib/util_tsock.c
lib/util_transfer_file.c
lib/sys_popen.c
- lib/sock_exec.c
''',
deps='''
CHARSET3
diff --git a/testsuite/build_farm/basicsmb.fns b/testsuite/build_farm/basicsmb.fns
index 3a9080f..d1127e8 100644
--- a/testsuite/build_farm/basicsmb.fns
+++ b/testsuite/build_farm/basicsmb.fns
@@ -177,8 +177,6 @@ test_listfilesauth_should_deny() {
return 0
}
-echo "LIBSMB_PROG=$LIBSMB_PROG" >&2
-
# Give sensible defaults to some variables.
--
2.7.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20171117/2adc148a/signature.sig>
More information about the samba-technical
mailing list