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.
---
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 +--
15 files changed, 69 insertions(+), 79 deletions(-)
(limited to 'src')
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