[LWN Logo]
[LWN.net]
From:	 Juanjo Ciarlante <jjo@mendoza.gov.ar>
To:	 BugTraq <bugtraq@securityfocus.com>
Subject: Re: [RAZOR] Linux kernel IP masquerading vulnerability (_actual_ patch)
Date:	 Mon, 30 Jul 2001 22:42:00 -0300

On Mon, Jul 30 2001 12:49:51 Michal Zalewski wrote:
> Topic:
>    A remotely exploitable IP masquerading vulnerability in the Linux
>    kernel can be used to penetrate protected private networks.
> :
> Vendor Response/Fix Information:
> 
> Below is a patch against Linux 2.2.20pre kernel written by the I
> masquerading subsystem maintainer, Juanjo Ciarlante
> ... 
The _actual_ working patch is attached, please apply this one, and of
course read original post by M. Zalewski: 
  http://www.securityfocus.com/archive/1/200361

--JuanJo Ciarlante
Linux IP MASQ 2.2 maintainer
	 

--- linux/net/ipv4/ip_masq_irc.c.dist	Sun Mar 25 13:31:12 2001
+++ linux/net/ipv4/ip_masq_irc.c	Mon Jul 30 13:29:49 2001
@@ -22,6 +22,7 @@
  *	  <sshore@escape.ca>
  *	Scottie Shore		:  added support for mIRC DCC resume negotiation
  *	  <sshore@escape.ca>
+ *	Juan Jose Ciarlante	:  src addr/port checking for better security (spotted by Michal Zalewski)
  *
  *	This program is free software; you can redistribute it and/or
  *	modify it under the terms of the GNU General Public License
@@ -38,7 +39,12 @@
  *	/etc/conf.modules (or /etc/modules.conf depending on your config)
  *	where modload will pick it up should you use modload to load your
  *	modules.
- *	
+ *
+ * Insecure "back" data channel opening
+ * 	The helper does some trivial checks when opening a new DCC data
+ * 	channel. Use module parameter
+ * 		insecure=1
+ *	... to avoid this and get previous (pre 2.2.20) behaviour.
  */
 
 #include <linux/config.h>
@@ -72,6 +78,9 @@
 
 MODULE_PARM(ports, "1-" __MODULE_STRING(MAX_MASQ_APP_PORTS) "i");
 
+static int insecure=0;
+MODULE_PARM(insecure, "i");
+
 
 /*
  * List of supported DCC protocols
@@ -110,6 +119,30 @@
         return 0;
 }
 
+
+/*
+ * 	Ugly workaround [TM] --mummy ... why does this protocol sucks?
+ *
+ *	The <1024 check and same source address just raise the
+ *	security "feeling" => they don't prevent a redirector listening
+ *	in same src address at a higher port; you should protect 
+ *	your internal network with ipchains output rules anyway
+ */
+
+static inline int masq_irc_out_check(const struct ip_masq *ms, __u32 data_saddr, __u16 data_sport) {
+	int allow=1;
+
+	IP_MASQ_DEBUG(1-debug, "masq_irc_out_check( s_addr=%d.%d.%d.%d, data_saddr=%d.%d.%d.%d, data_sport=%d",
+			NIPQUAD(ms->saddr), NIPQUAD(data_saddr), ntohs(data_sport));
+
+	/* 
+	 * 	Ignore data channel back to other src addr, nor to port < 1024
+	 */
+	if (ms->saddr != data_saddr || ntohs(data_sport) < 1024) 
+		allow=0;
+
+	return allow;
+}
 int
 masq_irc_out (struct ip_masq_app *mapp, struct ip_masq *ms, struct sk_buff **skb_p, __u32 maddr)
 {
@@ -118,7 +151,7 @@
 	struct tcphdr *th;
 	char *data, *data_limit;
 	__u32 s_addr;
-	__u16 s_port;
+	__u32 s_port;	/* larger to allow strtoul() return value validation */
 	struct ip_masq *n_ms;
 	char buf[20];		/* "m_addr m_port" (dec base)*/
         unsigned buf_len;
@@ -199,12 +232,25 @@
 				s_port = simple_strtoul(data,&data,10);
 				addr_end_p = data;
 
+				/*	Sanity checks 		*/
+				if (!s_addr || !s_port || s_port > 65535)
+					continue;
+
+				/*	Prefer net order from now on 	*/
+				s_addr = htonl(s_addr);
+				s_port = htons(s_port);
+
+				/* 	Simple validation 	*/
+				if (!insecure && !masq_irc_out_check(ms, s_addr, s_port))
+					/* We may just: return 0; */
+					continue;
+
 				/*	Do we already have a port open for this client?
 				 *	If so, use it (for DCC ACCEPT)
 				 */
 
 				n_ms = ip_masq_out_get(IPPROTO_TCP,
-						htonl(s_addr),htons(s_port),
+						s_addr, s_port,
 						0, 0);
 
 				/*
@@ -216,7 +262,7 @@
 				if (n_ms==NULL)
 					n_ms = ip_masq_new(IPPROTO_TCP,
 							maddr, 0,
-							htonl(s_addr),htons(s_port),
+							s_addr, s_port,
 							0, 0,
 							IP_MASQ_F_NO_DPORT|IP_MASQ_F_NO_DADDR);
 				if (n_ms==NULL)
@@ -236,7 +282,10 @@
 				diff = buf_len - (addr_end_p-addr_beg_p);
 
 				*addr_beg_p = '\0';
-				IP_MASQ_DEBUG(1-debug, "masq_irc_out(): '%s' %X:%X detected (diff=%d)\n", dcc_p, s_addr,s_port, diff);
+				IP_MASQ_DEBUG(1-debug, "masq_irc_out(): '%s' %d.%d.%d.%d:%d -> %d.%d.%d.%d:%d detected (diff=%d)\n", dcc_p, 
+					NIPQUAD(s_addr), htons(s_port), 
+					NIPQUAD(n_ms->maddr), htons(n_ms->mport), 
+					diff);
 
 				/*
 				 *	No shift.