Skip to content
Permalink
Browse files Browse the repository at this point in the history
In the open request, reject capture sources that are URLs.
You shouldn't be able to ask a server to open a remote device on some
*other* server; just open it yourself.

This addresses Include Security issue F13: [libpcap] Remote Packet
Capture Daemon Allows Opening Capture URLs.
  • Loading branch information
guyharris committed Sep 30, 2019
1 parent 484d60c commit 33834cb
Showing 1 changed file with 72 additions and 2 deletions.
74 changes: 72 additions & 2 deletions rpcapd/daemon.c
Expand Up @@ -156,6 +156,8 @@ static int rpcapd_recv(SOCKET sock, char *buffer, size_t toread, uint32 *plen, c
static int rpcapd_discard(SOCKET sock, uint32 len);
static void session_close(struct session *);

static int is_url(const char *source);

int
daemon_serviceloop(SOCKET sockctrl, int isactive, char *passiveClients,
int nullAuthAllowed)
Expand Down Expand Up @@ -1554,8 +1556,13 @@ daemon_msg_open_req(uint8 ver, struct daemon_slpars *pars, uint32 plen,
source[nread] = '\0';
plen -= nread;

// XXX - make sure it's *not* a URL; we don't support opening
// remote devices here.
// Is this a URL rather than a device?
// If so, reject it.
if (is_url(source))
{
pcap_snprintf(errmsgbuf, PCAP_ERRBUF_SIZE, "Source string refers to a remote device");
goto error;
}

// Open the selected device
// This is a fake open, since we do that only to get the needed parameters, then we close the device again
Expand Down Expand Up @@ -2647,3 +2654,66 @@ static void session_close(struct session *session)
session->fp = NULL;
}
}

// Check whether a capture source string is a URL or not.
// This includes URLs that refer to a local device; a scheme, followed
// by ://, followed by *another* scheme and ://, is just silly, and
// anybody who supplies that will get an error.
//
static int
is_url(const char *source)
{
char *colonp;

/*
* RFC 3986 says:
*
* URI = scheme ":" hier-part [ "?" query ] [ "#" fragment ]
*
* hier-part = "//" authority path-abempty
* / path-absolute
* / path-rootless
* / path-empty
*
* authority = [ userinfo "@" ] host [ ":" port ]
*
* userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
*
* Step 1: look for the ":" at the end of the scheme.
* A colon in the source is *NOT* sufficient to indicate that
* this is a URL, as interface names on some platforms might
* include colons (e.g., I think some Solaris interfaces
* might).
*/
colonp = strchr(source, ':');
if (colonp == NULL)
{
/*
* The source is the device to open. It's not a URL.
*/
return (0);
}

/*
* All schemes must have "//" after them, i.e. we only support
* hier-part = "//" authority path-abempty, not
* hier-part = path-absolute
* hier-part = path-rootless
* hier-part = path-empty
*
* We need that in order to distinguish between a local device
* name that happens to contain a colon and a URI.
*/
if (strncmp(colonp + 1, "//", 2) != 0)
{
/*
* The source is the device to open. It's not a URL.
*/
return (0);
}

/*
* It's a URL.
*/
return (1);
}

0 comments on commit 33834cb

Please sign in to comment.