RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-05-22 20:25:07 +00:00
Code Issues Activity Actions

Kernel: Don't use a VLA for outgoing TCP packets

Browse source 
Since the payload size is user-controlled, this could be used to
overflow the kernel stack.

We should probably also be breaking things into smaller packets at a
higher level, e.g TCPSocket::protocol_send(), but let's do that as
a separate exercise.

Fixes #5310.
This commit is contained in:
Andreas Kling 2021-02-12 22:56:54 +01:00
parent e47af3044a
commit da981578e3
1 changed files with 4 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

9
Kernel/Net/TCPSocket.cpp
View file

@ -187,9 +187,8 @@ KResultOr<size_t> TCPSocket::protocol_send(const UserOrKernelBuffer& data, size_
KResult TCPSocket::send_tcp_packet(u16 flags, const UserOrKernelBuffer* payload, size_t payload_size) KResult TCPSocket::send_tcp_packet(u16 flags, const UserOrKernelBuffer* payload, size_t payload_size)
{ {
const size_t buffer_size = sizeof(TCPPacket) + payload_size; const size_t buffer_size = sizeof(TCPPacket) + payload_size;
alignas(TCPPacket) u8 buffer[buffer_size]; auto buffer = ByteBuffer::create_zeroed(buffer_size);
new (buffer) TCPPacket; auto& tcp_packet = *(TCPPacket*)(buffer.data());
auto& tcp_packet = *(TCPPacket*)(buffer);
ASSERT(local_port()); ASSERT(local_port());
tcp_packet.set_source_port(local_port()); tcp_packet.set_source_port(local_port());
tcp_packet.set_destination_port(peer_port()); tcp_packet.set_destination_port(peer_port());
@ -214,7 +213,7 @@ KResult TCPSocket::send_tcp_packet(u16 flags, const UserOrKernelBuffer* payload,
if (tcp_packet.has_syn() || payload_size > 0) { if (tcp_packet.has_syn() || payload_size > 0) {
LOCKER(m_not_acked_lock); LOCKER(m_not_acked_lock);
m_not_acked.append({ m_sequence_number, ByteBuffer::copy(buffer, buffer_size) }); m_not_acked.append({ m_sequence_number, move(buffer) });
send_outgoing_packets(); send_outgoing_packets();
return KSuccess; return KSuccess;
} }
@ -222,7 +221,7 @@ KResult TCPSocket::send_tcp_packet(u16 flags, const UserOrKernelBuffer* payload,
auto routing_decision = route_to(peer_address(), local_address(), bound_interface()); auto routing_decision = route_to(peer_address(), local_address(), bound_interface());
ASSERT(!routing_decision.is_zero()); ASSERT(!routing_decision.is_zero());
auto packet_buffer = UserOrKernelBuffer::for_kernel_buffer(buffer); auto packet_buffer = UserOrKernelBuffer::for_kernel_buffer(buffer.data());
auto result = routing_decision.adapter->send_ipv4( auto result = routing_decision.adapter->send_ipv4(
routing_decision.next_hop, peer_address(), IPv4Protocol::TCP, routing_decision.next_hop, peer_address(), IPv4Protocol::TCP,
packet_buffer, buffer_size, ttl()); packet_buffer, buffer_size, ttl());