From: Guy Harris Date: Mon, 6 Aug 2018 03:05:52 +0000 (-0700) Subject: In the open request, reject capture sources that are URLs. X-Git-Tag: libpcap-1.10-bp~424 X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/9df01dba7d4a698dbfec57e678e5a73dae93fa6d?ds=sidebyside 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. --- diff --git a/rpcapd/daemon.c b/rpcapd/daemon.c index 76d22f28..831ecbfc 100644 --- a/rpcapd/daemon.c +++ b/rpcapd/daemon.c @@ -197,6 +197,8 @@ struct tls_alert { #define TLS_ALERT_LEVEL_FATAL 2 #define TLS_ALERT_HANDSHAKE_FAILURE 40 +static int is_url(const char *source); + int daemon_serviceloop(SOCKET sockctrl, int isactive, char *passiveClients, int nullAuthAllowed, int uses_ssl) @@ -1791,8 +1793,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)) + { + 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 @@ -2950,3 +2957,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); +}