mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 08:32:43 +00:00 
			
		
		
		
	Kernel: Stop modifying peer address/port in sendto on a TCP socket
POSIX (rightfully so) specifies that the sendto address argument is ignored in connection-oriented protocols. The TCPSocket also assumed the peer address may not change post-connect and would trigger a UAF in sockets_by_tuple() when it did.
This commit is contained in:
		
							parent
							
								
									8bb423daf7
								
							
						
					
					
						commit
						da2f33df82
					
				
					 3 changed files with 87 additions and 2 deletions
				
			
		|  | @ -204,9 +204,11 @@ ErrorOr<size_t> IPv4Socket::sendto(OpenFileDescription&, UserOrKernelBuffer cons | ||||||
|             return set_so_error(EAFNOSUPPORT); |             return set_so_error(EAFNOSUPPORT); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|  |         if (type() != SOCK_STREAM) { | ||||||
|             m_peer_address = IPv4Address((u8 const*)&ia.sin_addr.s_addr); |             m_peer_address = IPv4Address((u8 const*)&ia.sin_addr.s_addr); | ||||||
|             m_peer_port = ntohs(ia.sin_port); |             m_peer_port = ntohs(ia.sin_port); | ||||||
|         } |         } | ||||||
|  |     } | ||||||
| 
 | 
 | ||||||
|     if (!is_connected() && m_peer_address.is_zero()) |     if (!is_connected() && m_peer_address.is_zero()) | ||||||
|         return set_so_error(EPIPE); |         return set_so_error(EPIPE); | ||||||
|  |  | ||||||
|  | @ -56,6 +56,7 @@ set(LIBTEST_BASED_SOURCES | ||||||
|     TestSigAltStack.cpp |     TestSigAltStack.cpp | ||||||
|     TestSigHandler.cpp |     TestSigHandler.cpp | ||||||
|     TestSigWait.cpp |     TestSigWait.cpp | ||||||
|  |     TestTCPSocket.cpp | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| if (NOT CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64") | if (NOT CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64") | ||||||
|  |  | ||||||
							
								
								
									
										82
									
								
								Tests/Kernel/TestTCPSocket.cpp
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										82
									
								
								Tests/Kernel/TestTCPSocket.cpp
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,82 @@ | ||||||
|  | /*
 | ||||||
|  |  * Copyright (c) 2023, Idan Horowitz <idan.horowitz@serenityos.org> | ||||||
|  |  * | ||||||
|  |  * SPDX-License-Identifier: BSD-2-Clause | ||||||
|  |  */ | ||||||
|  | 
 | ||||||
|  | #include <LibTest/TestCase.h> | ||||||
|  | #include <netinet/in.h> | ||||||
|  | #include <pthread.h> | ||||||
|  | #include <sys/socket.h> | ||||||
|  | 
 | ||||||
|  | static constexpr u16 port = 1337; | ||||||
|  | 
 | ||||||
|  | static void* server_handler(void*) | ||||||
|  | { | ||||||
|  |     int server_fd = socket(AF_INET, SOCK_STREAM, 0); | ||||||
|  |     EXPECT(server_fd >= 0); | ||||||
|  | 
 | ||||||
|  |     sockaddr_in sin {}; | ||||||
|  |     sin.sin_family = AF_INET; | ||||||
|  |     sin.sin_port = htons(port); | ||||||
|  |     sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK); | ||||||
|  |     int rc = bind(server_fd, (sockaddr*)(&sin), sizeof(sin)); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  | 
 | ||||||
|  |     rc = listen(server_fd, 1); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  | 
 | ||||||
|  |     int client_fd = accept(server_fd, nullptr, nullptr); | ||||||
|  |     EXPECT(client_fd >= 0); | ||||||
|  | 
 | ||||||
|  |     u8 data; | ||||||
|  |     int nread = recv(client_fd, &data, sizeof(data), 0); | ||||||
|  |     EXPECT_EQ(nread, 1); | ||||||
|  |     EXPECT_EQ(data, 'A'); | ||||||
|  | 
 | ||||||
|  |     rc = close(client_fd); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  | 
 | ||||||
|  |     rc = close(server_fd); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  | 
 | ||||||
|  |     pthread_exit(nullptr); | ||||||
|  |     VERIFY_NOT_REACHED(); | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | static pthread_t start_tcp_server() | ||||||
|  | { | ||||||
|  |     pthread_t thread; | ||||||
|  |     int rc = pthread_create(&thread, nullptr, server_handler, nullptr); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  |     return thread; | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | TEST_CASE(tcp_sendto) | ||||||
|  | { | ||||||
|  |     pthread_t server = start_tcp_server(); | ||||||
|  | 
 | ||||||
|  |     int client_fd = socket(AF_INET, SOCK_STREAM, 0); | ||||||
|  |     EXPECT(client_fd >= 0); | ||||||
|  | 
 | ||||||
|  |     sockaddr_in sin {}; | ||||||
|  |     sin.sin_family = AF_INET; | ||||||
|  |     sin.sin_port = htons(port); | ||||||
|  |     sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK); | ||||||
|  |     int rc = connect(client_fd, (sockaddr*)(&sin), sizeof(sin)); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  | 
 | ||||||
|  |     u8 data = 'A'; | ||||||
|  |     sockaddr_in dst {}; | ||||||
|  |     dst.sin_family = AF_INET; | ||||||
|  |     dst.sin_port = htons(port + 1); // Different port, should be ignored
 | ||||||
|  |     dst.sin_addr.s_addr = htonl(INADDR_LOOPBACK); | ||||||
|  |     int nwritten = sendto(client_fd, &data, sizeof(data), 0, (sockaddr*)(&dst), sizeof(dst)); | ||||||
|  |     EXPECT_EQ(nwritten, 1); | ||||||
|  | 
 | ||||||
|  |     rc = close(client_fd); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  | 
 | ||||||
|  |     rc = pthread_join(server, nullptr); | ||||||
|  |     EXPECT_EQ(rc, 0); | ||||||
|  | } | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Idan Horowitz
						Idan Horowitz