From 9e41e71dfa6b1eb7d76d62fd4fc6ba49b5106ad2 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Tue, 29 Dec 2020 13:28:48 +0100 Subject: [PATCH] Add buffer size checks to Net::Arp --- src/net-arp.cpp | 137 ++++++++++++++++++++++++++---------------------- src/net-arp.h | 38 +++++++------- 2 files changed, 94 insertions(+), 81 deletions(-) diff --git a/src/net-arp.cpp b/src/net-arp.cpp index 3699556..232ee1e 100644 --- a/src/net-arp.cpp +++ b/src/net-arp.cpp @@ -4,24 +4,30 @@ #include "net-arp.h" #include "net-ethernet.h" +#include "debug.h" #include "types.h" #include namespace Net::Arp { - Packet::Packet() - {} + Packet::Packet() {} - Packet::Packet(uint16_t operation) : + Packet::Packet(const uint16_t operation) : hardwareType(1), // Ethernet protocolType(Ethernet::EtherType::Ipv4), hardwareAddressLength(6), protocolAddressLength(4), operation(operation) - {} - - size_t Packet::Serialize(uint8_t* buffer) { + } + + size_t Packet::Serialize(uint8_t* buffer, const size_t bufferSize) + { + if (bufferSize < SerializedLength()) + { + return 0; + } + buffer[0] = hardwareType >> 8; buffer[1] = hardwareType; buffer[2] = static_cast(protocolType) >> 8; @@ -49,33 +55,33 @@ namespace Net::Arp } // Static - Packet Packet::Deserialize(const uint8_t* buffer) + size_t Packet::Deserialize(const uint8_t* buffer, const size_t bufferSize) { - Packet self; + if (bufferSize < SerializedLength()) + { + return 0; + } - self.hardwareType = buffer[0] << 8 | buffer[1]; - self.protocolType = - static_cast(buffer[2] << 8 | buffer[3]); - self.hardwareAddressLength = buffer[4]; - self.protocolAddressLength = buffer[5]; - self.operation = buffer[6] << 8 | buffer[7]; + hardwareType = buffer[0] << 8 | buffer[1]; + protocolType = static_cast(buffer[2] << 8 | buffer[3]); + hardwareAddressLength = buffer[4]; + protocolAddressLength = buffer[5]; + operation = buffer[6] << 8 | buffer[7]; - memcpy(self.senderMac.data(), buffer + 8, 6); - self.senderIp = - buffer[14] << 24 | buffer[15] << 16 | buffer[16] << 8 | buffer[17]; - memcpy(self.targetMac.data(), buffer + 18, 6); - self.targetIp = - buffer[24] << 24 | buffer[25] << 16 | buffer[26] << 8 | buffer[27]; + memcpy(senderMac.data(), buffer + 8, 6); + senderIp = buffer[14] << 24 | buffer[15] << 16 | buffer[16] << 8 | buffer[17]; + memcpy(targetMac.data(), buffer + 18, 6); + targetIp = buffer[24] << 24 | buffer[25] << 16 | buffer[26] << 8 | buffer[27]; - return self; + return 28; } void SendPacket( - Operation operation, - Utils::MacAddress targetMac, - Utils::MacAddress senderMac, - uint32_t targetIp, - uint32_t senderIp) + const Operation operation, + const Utils::MacAddress targetMac, + const Utils::MacAddress senderMac, + const uint32_t targetIp, + const uint32_t senderIp) { Packet arpPacket(operation); arpPacket.targetMac = targetMac; @@ -83,16 +89,14 @@ namespace Net::Arp arpPacket.targetIp = targetIp; arpPacket.senderIp = senderIp; - Ethernet::Header ethernetHeader( - senderMac, targetMac, Ethernet::EtherType::Arp); + Ethernet::Header ethernetHeader(senderMac, targetMac, Ethernet::EtherType::Arp); uint8_t buffer[USPI_FRAME_BUFFER_SIZE]; size_t size = 0; size += ethernetHeader.Serialize(buffer + size, sizeof(buffer) - size); - size += arpPacket.Serialize(buffer + size); + size += arpPacket.Serialize(buffer + size, sizeof(buffer) - size); - const auto expectedSize = - ethernetHeader.SerializedLength() + arpPacket.SerializedLength(); + const auto expectedSize = ethernetHeader.SerializedLength() + arpPacket.SerializedLength(); assert(size == expectedSize); assert(size <= sizeof(buffer)); @@ -100,56 +104,65 @@ namespace Net::Arp } void SendRequest( - Utils::MacAddress targetMac, - Utils::MacAddress senderMac, - uint32_t targetIp, - uint32_t senderIp - ) { + const Utils::MacAddress targetMac, + const Utils::MacAddress senderMac, + const uint32_t targetIp, + const uint32_t senderIp) + { SendPacket(ARP_OPERATION_REQUEST, targetMac, senderMac, targetIp, senderIp); } void SendReply( - Utils::MacAddress targetMac, - Utils::MacAddress senderMac, - uint32_t targetIp, - uint32_t senderIp - ) { + const Utils::MacAddress targetMac, + const Utils::MacAddress senderMac, + const uint32_t targetIp, + const uint32_t senderIp) + { SendPacket(ARP_OPERATION_REPLY, targetMac, senderMac, targetIp, senderIp); } - void SendAnnouncement(Utils::MacAddress mac, uint32_t ip) + void SendAnnouncement(const Utils::MacAddress mac, const uint32_t ip) { SendReply(Utils::MacBroadcast, mac, ip, ip); } void HandlePacket( - const Ethernet::Header ethernetHeader, uint8_t* buffer - ) { + const Ethernet::Header ethernetHeader, const uint8_t* buffer, const size_t bufferSize) + { const auto macAddress = Utils::GetMacAddress(); - const auto arpPacket = Packet::Deserialize(buffer); - if ( - arpPacket.hardwareType == 1 && - arpPacket.protocolType == Ethernet::EtherType::Ipv4 && - arpPacket.operation == ARP_OPERATION_REQUEST && - arpPacket.targetIp == Utils::Ipv4Address) + Packet arpPacket; + size_t arpSize = arpPacket.Deserialize(buffer, bufferSize); + if (arpSize == 0 || arpSize != arpPacket.SerializedLength()) { - SendReply( - arpPacket.senderMac, - macAddress, - arpPacket.senderIp, - Utils::Ipv4Address - ); + DEBUG_LOG( + "Dropped ARP packet (invalid buffer size %lu, expected %lu)\r\n", + bufferSize, + arpPacket.SerializedLength()); + return; } - else if ( - arpPacket.hardwareType == 1 && - arpPacket.protocolType == Ethernet::EtherType::Ipv4 && - arpPacket.operation == ARP_OPERATION_REPLY && - arpPacket.targetIp == Utils::Ipv4Address && - arpPacket.targetMac == macAddress) + if (arpPacket.hardwareType != 1 || arpPacket.protocolType != Ethernet::EtherType::Ipv4 || + arpPacket.targetIp != Utils::Ipv4Address) { + // Might want to disable because of spamminess + DEBUG_LOG("Dropped ARP packet (invalid parameters)\r\n"); + return; + } + + switch (arpPacket.operation) + { + case ARP_OPERATION_REQUEST: + SendReply(arpPacket.senderMac, macAddress, arpPacket.senderIp, Utils::Ipv4Address); + break; + + case ARP_OPERATION_REPLY: ArpTable.insert(std::make_pair(arpPacket.senderIp, arpPacket.senderMac)); + break; + + default: + DEBUG_LOG("Dropped ARP packet (invalid operation %d)\r\n", arpPacket.operation); + break; } } diff --git a/src/net-arp.h b/src/net-arp.h index 8dd4b25..68ef023 100644 --- a/src/net-arp.h +++ b/src/net-arp.h @@ -25,7 +25,7 @@ namespace Net::Arp uint32_t targetIp; Packet(); - Packet(uint16_t operation); + Packet(const uint16_t operation); constexpr size_t SerializedLength() const { @@ -41,36 +41,36 @@ namespace Net::Arp sizeof(targetIp); } - size_t Serialize(uint8_t* buffer); - - static Packet Deserialize(const uint8_t* buffer); + size_t Serialize(uint8_t* buffer, const size_t bufferSize); + size_t Deserialize(const uint8_t* buffer, const size_t bufferSize); }; - void HandlePacket(Ethernet::Header header, uint8_t* buffer); + void HandlePacket( + const Ethernet::Header header, const uint8_t* buffer, const size_t bufferSize); void SendPacket( - Operation operation, - Utils::MacAddress targetMac, - Utils::MacAddress senderMac, - uint32_t senderIp, - uint32_t targetIp + const Operation operation, + const Utils::MacAddress targetMac, + const Utils::MacAddress senderMac, + const uint32_t senderIp, + const uint32_t targetIp ); void SendRequest( - Utils::MacAddress targetMac, - Utils::MacAddress senderMac, - uint32_t senderIp, - uint32_t targetIp + const Utils::MacAddress targetMac, + const Utils::MacAddress senderMac, + const uint32_t senderIp, + const uint32_t targetIp ); void SendReply( - Utils::MacAddress targetMac, - Utils::MacAddress senderMac, - uint32_t senderIp, - uint32_t targetIp + const Utils::MacAddress targetMac, + const Utils::MacAddress senderMac, + const uint32_t senderIp, + const uint32_t targetIp ); - void SendAnnouncement(Utils::MacAddress mac, uint32_t ip); + void SendAnnouncement(const Utils::MacAddress mac, const uint32_t ip); extern std::unordered_map ArpTable; } // namespace Net::Arp