diff --git a/scripts/base/frameworks/dpd/dpd.sig b/scripts/base/frameworks/dpd/dpd.sig index 245e79bfdf..05dcee9d49 100644 --- a/scripts/base/frameworks/dpd/dpd.sig +++ b/scripts/base/frameworks/dpd/dpd.sig @@ -194,14 +194,16 @@ signature dpd_socks4_reverse_server { signature dpd_socks5_client { ip-proto == tcp - payload /^\x05/ + # Watch for a few authentication methods to reduce false positives. + payload /^\x05.[\x00\x01\x02]/ tcp-state originator } signature dpd_socks5_server { ip-proto == tcp requires-reverse-signature dpd_socks5_client - payload /^\x05/ + # Watch for a single authentication method to be chosen by the server. + payload /^\x05\x01[\x00\x01\x02]/ tcp-state responder enable "socks" } diff --git a/scripts/base/protocols/socks/consts.bro b/scripts/base/protocols/socks/consts.bro index 6341262041..fb14aae601 100644 --- a/scripts/base/protocols/socks/consts.bro +++ b/scripts/base/protocols/socks/consts.bro @@ -11,7 +11,6 @@ export { [1] = "GSSAPI", [2] = "Username/Password", [3] = "Challenge-Handshake Authentication Protocol", - [4] = "Unassigned", [5] = "Challenge-Response Authentication Method", [6] = "Secure Sockets Layer", [7] = "NDS Authentication", diff --git a/src/socks-analyzer.pac b/src/socks-analyzer.pac index 2c3fd68e09..1d71753fb5 100644 --- a/src/socks-analyzer.pac +++ b/src/socks-analyzer.pac @@ -22,6 +22,7 @@ refine connection SOCKS_Conn += { function socks4_request(request: SOCKS4_Request): bool %{ + StringVal *dstname = 0; if ( ${request.v4a} ) dstname = array_to_string(${request.name}); @@ -59,6 +60,12 @@ refine connection SOCKS_Conn += { function socks5_request(request: SOCKS5_Request): bool %{ + if ( ${request.reserved} != 0 ) + { + bro_analyzer()->ProtocolViolation(fmt("invalid value in reserved field: %d", ${request.reserved})); + return false; + } + AddrVal *ip_addr = 0; StringVal *domain_name = 0; @@ -77,6 +84,11 @@ refine connection SOCKS_Conn += { case 4: ip_addr = new AddrVal(IPAddr(IPv6, (const uint32_t*) ${request.remote_name.ipv6}, IPAddr::Network)); break; + + default: + bro_analyzer()->ProtocolViolation(fmt("invalid SOCKSv5 addr type: %d", ${request.remote_name.addr_type})); + return false; + break; } if ( ! ip_addr ) @@ -118,6 +130,11 @@ refine connection SOCKS_Conn += { case 4: ip_addr = new AddrVal(IPAddr(IPv6, (const uint32_t*) ${reply.bound.ipv6}, IPAddr::Network)); break; + + default: + bro_analyzer()->ProtocolViolation(fmt("invalid SOCKSv5 addr type: %d", ${reply.bound.addr_type})); + return false; + break; } if ( ! ip_addr ) @@ -138,6 +155,17 @@ refine connection SOCKS_Conn += { return true; %} + function version_error(version: uint8): bool + %{ + bro_analyzer()->ProtocolViolation(fmt("unsupported/unknown SOCKS version %d", version)); + return true; + %} + + +}; + +refine typeattr SOCKS_Version_Error += &let { + proc: bool = $context.connection.version_error(version); }; refine typeattr SOCKS4_Request += &let { diff --git a/src/socks-protocol.pac b/src/socks-protocol.pac index 8ae81a6e02..0d441665f6 100644 --- a/src/socks-protocol.pac +++ b/src/socks-protocol.pac @@ -4,10 +4,14 @@ type SOCKS_Version(is_orig: bool) = record { msg: case version of { 4 -> socks4_msg: SOCKS4_Message(is_orig); 5 -> socks5_msg: SOCKS5_Message(is_orig); - default -> socks_msg_fail: empty; + default -> socks_msg_fail: SOCKS_Version_Error(version); }; }; +type SOCKS_Version_Error(version: uint8) = record { + nothing: empty; +}; + # SOCKS5 Implementation type SOCKS5_Message(is_orig: bool) = case $context.connection.v5_past_authentication() of { true -> msg: SOCKS5_Real_Message(is_orig);