From f88fbaa2fe91c22f1c7280ecd96c2bcf92f73a4d Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Thu, 11 Aug 2022 14:20:18 +0100 Subject: Move aptosa/untosa into the sockaddrs union and add from/from_ip. The struct will also now always be zero-initialized by default which removes the footgun which has happened previously where the union has been accessed before being initialized leading to it containing weird values. --- include/socket.h | 43 ++++++++++++++++--------- src/cidr.cpp | 4 +-- src/coremods/core_dns.cpp | 19 +++++------- src/listensocket.cpp | 2 +- src/modules/m_cloaking.cpp | 6 ++-- src/modules/m_gateway.cpp | 8 ++--- src/modules/m_ident.cpp | 7 ++--- src/modules/m_spanningtree/main.cpp | 6 ++-- src/modules/m_spanningtree/resolvers.cpp | 4 +-- src/modules/m_spanningtree/treesocket1.cpp | 3 +- src/modules/m_spanningtree/uid.cpp | 8 ++--- src/modules/m_spanningtree/utils.cpp | 4 +-- src/modules/m_websocket.cpp | 15 ++++----- src/serializable.cpp | 8 ++--- src/socket.cpp | 50 +++++++++++++++++------------- src/users.cpp | 4 +-- 16 files changed, 97 insertions(+), 94 deletions(-) diff --git a/include/socket.h b/include/socket.h index a490ebcaf..8347ef84d 100644 --- a/include/socket.h +++ b/include/socket.h @@ -56,12 +56,40 @@ namespace irc struct sockaddr_in6 in6; struct sockaddr_un un; + /** Initializes this sockaddrs optionally as an unspecified socket address. */ + explicit sockaddrs(bool initialize = true); + /** Returns the address segment of the socket address as a string. */ std::string addr() const; /** Returns the family of the socket address (e.g. AF_INET). */ int family() const; + /** Store an IP address or UNIX socket path in this socket address. + * @param addr An IPv4 address, IPv6 address, or UNIX socket path. + * @return True if the IP address or UNIX socket paht was stored in this socket address; otherwise, false. + */ + inline bool from(const std::string& addr) { return addr.find('/') == std::string::npos ? from_ip(addr) : from_unix(addr); } + + /** Store an IP address in this socket address. + * @param addr An IPv4 or IPv6 address. + * @return True if the IP was stored in this socket address; otherwise, false. + */ + inline bool from_ip(const std::string& addr) { return from_ip_port(addr, 0); } + + /** Store an IP address and TCP port pair in this socket address. + * @param addr An IPv4 or IPv6 address. + * @param port A TCP port. + * @return True if the IP/port was stored in this socket address; otherwise, false. + */ + bool from_ip_port(const std::string& addr, int port); + + /** Store a UNIX socket path in this socket address. + * @param path A path to a UNIX socket. + * @return True if the UNIX socket path was stored in this socket address; otherwise, false. + */ + bool from_unix(const std::string& path); + /** Returns the TCP port number of the socket address or 0 if not relevant to this family. */ int port() const; @@ -114,21 +142,6 @@ namespace irc */ CoreExport bool MatchCIDR(const std::string &address, const std::string &cidr_mask, bool match_with_username); - /** Convert an address-port pair into a binary sockaddr - * @param addr The IP address, IPv4 or IPv6 - * @param port The port, 0 for unspecified - * @param sa The structure to place the result in. Will be zeroed prior to conversion - * @return true if the conversion was successful, false if not. - */ - CoreExport bool aptosa(const std::string& addr, int port, irc::sockets::sockaddrs& sa); - - /** Convert a UNIX socket path to a binary sockaddr. - * @param path The path to the UNIX socket. - * @param sa The structure to place the result in. Will be zeroed prior to conversion. - * @return True if the conversion was successful; otherwise, false. - */ - CoreExport bool untosa(const std::string& path, irc::sockets::sockaddrs& sa); - /** Determines whether the specified file is a UNIX socket. * @param file The path to the file to check. * @return True if the file is a UNIX socket; otherwise, false. diff --git a/src/cidr.cpp b/src/cidr.cpp index f45c63b98..f7401fe92 100644 --- a/src/cidr.cpp +++ b/src/cidr.cpp @@ -76,8 +76,8 @@ bool irc::sockets::MatchCIDR(const std::string &address, const std::string &cidr return false; } - irc::sockets::sockaddrs addr; - if (!irc::sockets::aptosa(address_copy, 0, addr)) + irc::sockets::sockaddrs addr(false); + if (!addr.from_ip(address_copy)) { // The address could not be parsed. return false; diff --git a/src/coremods/core_dns.cpp b/src/coremods/core_dns.cpp index e61e745c1..d94199a8d 100644 --- a/src/coremods/core_dns.cpp +++ b/src/coremods/core_dns.cpp @@ -168,8 +168,6 @@ private: throw Exception(creator, "Unable to unpack A resource record"); irc::sockets::sockaddrs addrs; - memset(&addrs, 0, sizeof(addrs)); - addrs.in4.sin_family = AF_INET; addrs.in4.sin_addr.s_addr = input[pos] | (input[pos + 1] << 8) | (input[pos + 2] << 16) | (input[pos + 3] << 24); pos += 4; @@ -183,8 +181,6 @@ private: throw Exception(creator, "Unable to unpack AAAA resource record"); irc::sockets::sockaddrs addrs; - memset(&addrs, 0, sizeof(addrs)); - addrs.in6.sin6_family = AF_INET6; for (int j = 0; j < 16; ++j) addrs.in6.sin6_addr.s6_addr[j] = input[pos + j]; @@ -333,8 +329,8 @@ public: if (q.type == QUERY_PTR) { - irc::sockets::sockaddrs ip; - if (!irc::sockets::aptosa(q.name, 0, ip)) + irc::sockets::sockaddrs ip(false); + if (!ip.from_ip(q.name)) throw Exception(creator, "Unable to pack packet with malformed IP for PTR lookup"); if (q.name.find(':') != std::string::npos) @@ -637,7 +633,7 @@ public: void OnEventHandlerRead() override { unsigned char buffer[524]; - irc::sockets::sockaddrs from; + irc::sockets::sockaddrs from(false); socklen_t x = sizeof(from); ssize_t length = SocketEngine::RecvFrom(this, buffer, sizeof(buffer), 0, &from.sa, &x); @@ -773,7 +769,7 @@ public: void Rehash(const std::string& dnsserver, std::string sourceaddr, unsigned int sourceport) { - irc::sockets::aptosa(dnsserver, DNS::PORT, myserver); + myserver.from_ip_port(dnsserver, DNS::PORT); /* Initialize mastersocket */ Close(); @@ -786,16 +782,17 @@ public: SocketEngine::SetReuse(s); SocketEngine::NonBlocking(s); - irc::sockets::sockaddrs bindto; if (sourceaddr.empty()) { - // set a sourceaddr for irc::sockets::aptosa() based on the servers af type + // set a sourceaddr based on the servers af type if (myserver.family() == AF_INET) sourceaddr = "0.0.0.0"; else if (myserver.family() == AF_INET6) sourceaddr = "::"; } - irc::sockets::aptosa(sourceaddr, sourceport, bindto); + + irc::sockets::sockaddrs bindto; + bindto.from_ip_port(sourceaddr, sourceport); if (SocketEngine::Bind(this->GetFd(), bindto) < 0) { diff --git a/src/listensocket.cpp b/src/listensocket.cpp index e2657f227..3a69b84ed 100644 --- a/src/listensocket.cpp +++ b/src/listensocket.cpp @@ -150,7 +150,7 @@ ListenSocket::~ListenSocket() void ListenSocket::OnEventHandlerRead() { - irc::sockets::sockaddrs client; + irc::sockets::sockaddrs client(false); irc::sockets::sockaddrs server(bind_sa); socklen_t length = sizeof(client); diff --git a/src/modules/m_cloaking.cpp b/src/modules/m_cloaking.cpp index b04295ef1..c8a8cfaa2 100644 --- a/src/modules/m_cloaking.cpp +++ b/src/modules/m_cloaking.cpp @@ -509,8 +509,8 @@ public: { std::string chost; - irc::sockets::sockaddrs hostip; - bool host_is_ip = irc::sockets::aptosa(host, ip.port(), hostip) && hostip == ip; + irc::sockets::sockaddrs hostip(false); + bool host_is_ip = hostip.from_ip_port(host, ip.port()) && hostip == ip; switch (info.mode) { @@ -569,7 +569,7 @@ CmdResult CommandCloak::Handle(User* user, const Params& parameters) // If we're cloaking an IP address we pass it in the IP field too. irc::sockets::sockaddrs sa; - const char* ipaddr = irc::sockets::aptosa(parameters[0], 0, sa) ? parameters[0].c_str() : ""; + const char* ipaddr = sa.from_ip(parameters[0]) ? parameters[0].c_str() : ""; unsigned int id = 0; for (const auto& info : mod->cloaks) diff --git a/src/modules/m_gateway.cpp b/src/modules/m_gateway.cpp index a00d2d0df..d4a9a3d7b 100644 --- a/src/modules/m_gateway.cpp +++ b/src/modules/m_gateway.cpp @@ -140,8 +140,8 @@ public: CmdResult HandleLocal(LocalUser* user, const Params& parameters) override { - irc::sockets::sockaddrs sa; - if (irc::sockets::aptosa(parameters[0], 0, sa)) + irc::sockets::sockaddrs sa(false); + if (sa.from_ip(parameters[0])) { if (sa.family() != AF_INET) { @@ -263,8 +263,8 @@ public: if (!host.Matches(user, parameters[0], sslapi)) continue; - irc::sockets::sockaddrs ipaddr; - if (!irc::sockets::aptosa(parameters[3], user->client_sa.port(), ipaddr)) + irc::sockets::sockaddrs ipaddr(false); + if (!ipaddr.from_ip_port(parameters[3], user->client_sa.port())) { ServerInstance->SNO.WriteGlobalSno('w', "Connecting user %s (%s) tried to use WEBIRC but gave an invalid IP address.", user->uuid.c_str(), user->GetIPString().c_str()); diff --git a/src/modules/m_ident.cpp b/src/modules/m_ident.cpp index 6866af3d4..17b5c4c24 100644 --- a/src/modules/m_ident.cpp +++ b/src/modules/m_ident.cpp @@ -120,11 +120,8 @@ public: done = false; - irc::sockets::sockaddrs bindaddr; - irc::sockets::sockaddrs connaddr; - - memcpy(&bindaddr, &user->server_sa, sizeof(bindaddr)); - memcpy(&connaddr, &user->client_sa, sizeof(connaddr)); + irc::sockets::sockaddrs bindaddr(user->server_sa); + irc::sockets::sockaddrs connaddr(user->client_sa); if (connaddr.family() == AF_INET6) { diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp index ad45a2746..520868978 100644 --- a/src/modules/m_spanningtree/main.cpp +++ b/src/modules/m_spanningtree/main.cpp @@ -223,7 +223,7 @@ void ModuleSpanningTree::ConnectServer(std::shared_ptr x, std::shared_ptr< irc::sockets::sockaddrs sa; if (x->IPAddr.find('/') != std::string::npos) { - if (!irc::sockets::isunix(x->IPAddr) || !irc::sockets::untosa(x->IPAddr, sa)) + if (!irc::sockets::isunix(x->IPAddr) || !sa.from_unix(x->IPAddr)) { // We don't use the family() != AF_UNSPEC check below for UNIX sockets as // that results in a DNS lookup. @@ -235,7 +235,7 @@ void ModuleSpanningTree::ConnectServer(std::shared_ptr x, std::shared_ptr< else { // If this fails then the IP sa will be AF_UNSPEC. - irc::sockets::aptosa(x->IPAddr, x->Port, sa); + sa.from_ip_port(x->IPAddr, x->Port); } /* Do we already have an IP? If so, no need to resolve it. */ @@ -259,7 +259,7 @@ void ModuleSpanningTree::ConnectServer(std::shared_ptr x, std::shared_ptr< // Guess start_type from bindip aftype DNS::QueryType start_type = DNS::QUERY_AAAA; irc::sockets::sockaddrs bind; - if ((!x->Bind.empty()) && (irc::sockets::aptosa(x->Bind, 0, bind))) + if (!x->Bind.empty() && bind.from_ip(x->Bind)) { if (bind.family() == AF_INET) start_type = DNS::QUERY_A; diff --git a/src/modules/m_spanningtree/resolvers.cpp b/src/modules/m_spanningtree/resolvers.cpp index 4c449f2c4..e77db55a5 100644 --- a/src/modules/m_spanningtree/resolvers.cpp +++ b/src/modules/m_spanningtree/resolvers.cpp @@ -55,8 +55,8 @@ void ServernameResolver::OnLookupComplete(const DNS::Query *r) return; } - irc::sockets::sockaddrs sa; - if (!irc::sockets::aptosa(ans_record->rdata, MyLink->Port, sa)) + irc::sockets::sockaddrs sa(false); + if (!sa.from_ip_port(ans_record->rdata, MyLink->Port)) { // We had a result but it wasn't a valid IPv4/IPv6. OnError(r); diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp index 7cf1e170c..f66227084 100644 --- a/src/modules/m_spanningtree/treesocket1.cpp +++ b/src/modules/m_spanningtree/treesocket1.cpp @@ -48,10 +48,9 @@ TreeSocket::TreeSocket(std::shared_ptr link, std::shared_ptr capab->ac = myac; irc::sockets::sockaddrs bind; - memset(&bind, 0, sizeof(bind)); if (!link->Bind.empty() && (dest.family() == AF_INET || dest.family() == AF_INET6)) { - if (!irc::sockets::aptosa(link->Bind, 0, bind)) + if (!bind.from_ip(link->Bind)) { state = I_ERROR; SetError("Bind address '" + link->Bind + "' is not a valid IPv4 or IPv6 address"); diff --git a/src/modules/m_spanningtree/uid.cpp b/src/modules/m_spanningtree/uid.cpp index bd878d488..9fe9069a0 100644 --- a/src/modules/m_spanningtree/uid.cpp +++ b/src/modules/m_spanningtree/uid.cpp @@ -70,12 +70,8 @@ CmdResult CommandUID::HandleServer(TreeServer* remoteserver, CommandBase::Params } } - irc::sockets::sockaddrs sa; - if (params[6].find('/') != std::string::npos) - irc::sockets::untosa(params[6], sa); - else - irc::sockets::aptosa(params[6], 0, sa); - if (sa.family() == AF_UNSPEC) + irc::sockets::sockaddrs sa(false); + if (!sa.from(params[6])) throw ProtocolException("Invalid IP address or UNIX socket path"); /* For remote users, we pass the UUID they sent to the constructor. diff --git a/src/modules/m_spanningtree/utils.cpp b/src/modules/m_spanningtree/utils.cpp index 3afe0fddb..798df840f 100644 --- a/src/modules/m_spanningtree/utils.cpp +++ b/src/modules/m_spanningtree/utils.cpp @@ -212,8 +212,8 @@ void SpanningTreeUtilities::RefreshIPCache() ValidIPs.insert(ValidIPs.end(), L->AllowMasks.begin(), L->AllowMasks.end()); - irc::sockets::sockaddrs dummy; - bool ipvalid = irc::sockets::aptosa(L->IPAddr, L->Port, dummy); + irc::sockets::sockaddrs dummy(false); + bool ipvalid = dummy.from_ip_port(L->IPAddr, L->Port); if ((L->IPAddr == "*") || (isunix) || (ipvalid)) ValidIPs.push_back(L->IPAddr); else if (this->Creator->DNS) diff --git a/src/modules/m_websocket.cpp b/src/modules/m_websocket.cpp index fd5bd2a73..f9363f998 100644 --- a/src/modules/m_websocket.cpp +++ b/src/modules/m_websocket.cpp @@ -408,7 +408,12 @@ class WebSocketHook final if (proxyheader.Find(recvq, "X-Real-IP:", 10, reqend) || proxyheader.Find(recvq, "X-Forwarded-For:", 16, reqend)) { // Attempt to parse the proxy HTTP header. - irc::sockets::aptosa(proxyheader.ExtractValue(recvq), realsa.port(), realsa); + if (!realsa.from_ip_port(proxyheader.ExtractValue(recvq), realsa.port())) + { + // The proxy header value contains a malformed value. + FailHandshake(sock, "HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n", "WebSocket: Received a proxied HTTP request that sent a malformed real IP address"); + return -1; + } } else { @@ -416,14 +421,6 @@ class WebSocketHook final FailHandshake(sock, "HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n", "WebSocket: Received a proxied HTTP request that did not send a real IP address header"); return -1; } - - if (realsa.family() == AF_UNSPEC) - { - // The proxy header value contains a malformed value. - FailHandshake(sock, "HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n", "WebSocket: Received a proxied HTTP request that sent a malformed real IP address"); - return -1; - } - for (const auto& proxyrange : config.proxyranges) { if (InspIRCd::MatchCIDR(luser->GetIPString(), proxyrange, ascii_case_insensitive_map)) diff --git a/src/serializable.cpp b/src/serializable.cpp index cad72cf8c..57c71bd29 100644 --- a/src/serializable.cpp +++ b/src/serializable.cpp @@ -163,8 +163,8 @@ bool User::Deserialize(Serializable::Data& data) else oper = std::make_shared(user_oper); - irc::sockets::sockaddrs sa; - if (irc::sockets::aptosa(client_addr, client_port, sa) || irc::sockets::untosa(client_addr, sa)) + irc::sockets::sockaddrs sa(false); + if (sa.from_ip_port(client_addr, client_port) || sa.from_unix(client_addr)) client_sa = sa; InvalidateCache(); @@ -246,8 +246,8 @@ bool LocalUser::Deserialize(Serializable::Data& data) .Load("server_sa.port", server_port); // Apply the rest of the members. - irc::sockets::sockaddrs sa; - if (irc::sockets::aptosa(server_addr, server_port, sa) || irc::sockets::untosa(server_addr, sa)) + irc::sockets::sockaddrs sa(false); + if (sa.from_ip_port(server_addr, server_port) || sa.from_unix(server_addr)) server_sa = sa; // These are bitfields so we need to ensure they only get the appropriate bits. diff --git a/src/socket.cpp b/src/socket.cpp index fcf154037..56825eedd 100644 --- a/src/socket.cpp +++ b/src/socket.cpp @@ -88,8 +88,8 @@ size_t InspIRCd::BindPorts(FailedPortList& failed_ports) irc::portparser portrange(portlist, false); for (int port; (port = static_cast(portrange.GetToken())); ) { - irc::sockets::sockaddrs bindspec; - if (!irc::sockets::aptosa(address, port, bindspec)) + irc::sockets::sockaddrs bindspec(false); + if (!bindspec.from_ip_port(address, port)) continue; if (!BindPort(tag, bindspec, old_ports)) @@ -124,7 +124,7 @@ size_t InspIRCd::BindPorts(FailedPortList& failed_ports) continue; } - irc::sockets::untosa(fullpath, bindspec); + bindspec.from_unix(fullpath); if (!BindPort(tag, bindspec, old_ports)) failed_ports.emplace_back(errno, bindspec, tag); else @@ -154,46 +154,52 @@ size_t InspIRCd::BindPorts(FailedPortList& failed_ports) return bound; } -bool irc::sockets::aptosa(const std::string& addr, int port, irc::sockets::sockaddrs& sa) +irc::sockets::sockaddrs::sockaddrs(bool initialize) +{ + if (initialize) + memset(this, 0, sizeof(*this)); +} + +bool irc::sockets::sockaddrs::from_ip_port(const std::string& addr, int port) { - memset(&sa, 0, sizeof(sa)); if (addr.empty() || addr == "*") { if (ServerInstance->Config->WildcardIPv6) { - sa.in6.sin6_family = AF_INET6; - sa.in6.sin6_port = htons(port); + memset(&in6.sin6_addr, 0, sizeof(in6.sin6_addr)); + in6.sin6_family = AF_INET6; + in6.sin6_port = htons(port); } else { - sa.in4.sin_family = AF_INET; - sa.in4.sin_port = htons(port); + memset(&in4.sin_addr, 0, sizeof(in4.sin_addr)); + in4.sin_family = AF_INET; + in4.sin_port = htons(port); } return true; } - else if (inet_pton(AF_INET, addr.c_str(), &sa.in4.sin_addr) > 0) + else if (inet_pton(AF_INET, addr.c_str(), &in4.sin_addr) > 0) { - sa.in4.sin_family = AF_INET; - sa.in4.sin_port = htons(port); + in4.sin_family = AF_INET; + in4.sin_port = htons(port); return true; } - else if (inet_pton(AF_INET6, addr.c_str(), &sa.in6.sin6_addr) > 0) + else if (inet_pton(AF_INET6, addr.c_str(), &in6.sin6_addr) > 0) { - sa.in6.sin6_family = AF_INET6; - sa.in6.sin6_port = htons(port); + in6.sin6_family = AF_INET6; + in6.sin6_port = htons(port); return true; } return false; } -bool irc::sockets::untosa(const std::string& path, irc::sockets::sockaddrs& sa) +bool irc::sockets::sockaddrs::from_unix(const std::string& path) { - memset(&sa, 0, sizeof(sa)); - if (path.length() >= sizeof(sa.un.sun_path)) + if (path.length() >= sizeof(un.sun_path)) return false; - sa.un.sun_family = AF_UNIX; - memcpy(&sa.un.sun_path, path.c_str(), path.length() + 1); + un.sun_family = AF_UNIX; + memcpy(&un.sun_path, path.c_str(), path.length() + 1); return true; } @@ -382,13 +388,13 @@ irc::sockets::cidr_mask::cidr_mask(const std::string& mask) if (bits_chars == std::string::npos) { - irc::sockets::aptosa(mask, 0, sa); + sa.from_ip(mask); sa2cidr(*this, sa, 128); } else { unsigned char range = ConvToNum(mask.substr(bits_chars + 1)); - irc::sockets::aptosa(mask.substr(0, bits_chars), 0, sa); + sa.from_ip(mask.substr(0, bits_chars)); sa2cidr(*this, sa, range); } } diff --git a/src/users.cpp b/src/users.cpp index 37277643c..809fdb15a 100644 --- a/src/users.cpp +++ b/src/users.cpp @@ -96,8 +96,6 @@ User::User(const std::string& uid, Server* srv, Type type) , uniqueusername(false) , usertype(type) { - client_sa.sa.sa_family = AF_UNSPEC; - ServerInstance->Logs.Debug("USERS", "New UUID for user: %s", uuid.c_str()); if (srv->IsService()) @@ -114,6 +112,7 @@ User::User(const std::string& uid, Server* srv, Type type) LocalUser::LocalUser(int myfd, irc::sockets::sockaddrs* client, irc::sockets::sockaddrs* servaddr) : User(ServerInstance->UIDGen.GetUID(), ServerInstance->FakeClient->server, User::TYPE_LOCAL) , eh(this) + , server_sa(*servaddr) , quitting_sendq(false) , lastping(true) , exempt(false) @@ -124,7 +123,6 @@ LocalUser::LocalUser(int myfd, irc::sockets::sockaddrs* client, irc::sockets::so ident = uuid; eh.SetFd(myfd); memcpy(&client_sa, client, sizeof(irc::sockets::sockaddrs)); - memcpy(&server_sa, servaddr, sizeof(irc::sockets::sockaddrs)); ChangeRealHost(GetIPString(), true); } -- cgit v1.3.1-10-gc9f91