⬢ DragonFlyBSD Kernel Audit
DF-5825 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/kern/kern_dmsg.c b/sys/kern/kern_dmsg.c
index d6e90ee..0678b80 100644
--- a/sys/kern/kern_dmsg.c
+++ b/sys/kern/kern_dmsg.c
@@ -983,6 +983,25 @@ again:
 			error = EINVAL;
 			break;
 		}
+		/*
+		 * A received REPLY can only be for a transaction that WE
+		 * initiated, which is tracked in statewr_tree.  The tree
+		 * used for the initial lookup is selected solely by the
+		 * attacker-controlled DMSGF_REVTRANS bit, so without this
+		 * check a peer can deliver a REPLY|CREATE message against
+		 * a state it initiated (staterd_tree, KDMSG_STATE_OPPOSITE)
+		 * and inject DMSGF_REPLY into rxcmd.  The subsequent DELETE
+		 * processing would then RB_REMOVE() the state from the
+		 * WRONG red-black tree, leaving a dangling tree root that
+		 * points at freed memory (use-after-free in every later
+		 * tree operation on the link).
+		 */
+		if (state->flags & KDMSG_STATE_OPPOSITE) {
+			kdio_printf(iocom, 1, "%s\n",
+				    "msgrx: REPLY on received state");
+			error = EINVAL;
+			break;
+		}
 		state->rxcmd = msg->any.head.cmd & ~DMSGF_DELETE;
 		error = 0;
 		break;
@@ -1003,6 +1022,25 @@ again:
 			break;
 		}
 
+		/*
+		 * Same protection as the REPLY|CREATE case above: a reply
+		 * can only be for a transaction WE initiated.  Without
+		 * this check a REPLY|DELETE against a received state runs
+		 * into the wrong-branch KKASSERT (or the wrong tree) in
+		 * the DELETE post-processing below.
+		 */
+		if (state->flags & KDMSG_STATE_OPPOSITE) {
+			if (msg->any.head.cmd & DMSGF_ABORT) {
+				error = EALREADY;
+			} else {
+				kdio_printf(iocom, 1, "%s\n",
+					    "msgrx: REPLY|DELETE on "
+					    "received state");
+				error = EINVAL;
+			}
+			break;
+		}
+
 		/*
 		 * Received REPLY+ABORT+DELETE in case where msgid has
 		 * already been reused for an unrelated message,
@@ -1077,7 +1115,12 @@ done:
 		state->rxcmd |= DMSGF_DELETE;
 		if (state->txcmd & DMSGF_DELETE) {
 			KKASSERT(state->flags & KDMSG_STATE_RBINSERTED);
-			if (state->rxcmd & DMSGF_REPLY) {
+			/*
+			 * Defense-in-depth: select the tree by which side
+			 * the state is on (KDMSG_STATE_OPPOSITE), not by
+			 * bits in rxcmd which a malicious peer can smear.
+			 */
+			if ((state->flags & KDMSG_STATE_OPPOSITE) == 0) {
 				KKASSERT(msg->any.head.cmd &
 					 DMSGF_REPLY);
 				RB_REMOVE(kdmsg_state_tree,