netdev - Re: [Security] TIPC security issues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20101027.105047.183059900.davem@davemloft.net>
Date:	Wed, 27 Oct 2010 10:50:47 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	torvalds@...ux-foundation.org
Cc:	drosenberg@...curity.com, jon.maloy@...csson.com,
	allan.stephens@...driver.com, netdev@...r.kernel.org,
	security@...nel.org
Subject: Re: [Security] TIPC security issues

From: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Wed, 27 Oct 2010 10:37:46 -0700

> If you _really_ care deeply, then some packet-oriented protocol can
> just have its own private packet size limit (which would be way less
> than 2GB), and then just look at the total size and say "oh, the total
> size is bigger than my limit, so I'll just error out". Then, the fact
> that verify_iovec() may have truncated the message to 2GB-1 doesn't
> matter at all.
> 
> (Practically speaking, I bet all packet-oriented protocols already
> have a limit that is enforced by simply allocation patterns, so I
> don't think it's actually a problem even now)

This is, as it turns out, effectively what the TIPC socket layer
already does.

Most of the send calls that propagate down to this code adding up the
iov_len lengths gets passed a maximum packet size.

Anyways, here is what I came up with to kill this specific bug in
TIPC:

>From 39dc17049a5ed989bab8997945a048ffddf48387 Mon Sep 17 00:00:00 2001
From: David S. Miller <davem@...emloft.net>
Date: Wed, 27 Oct 2010 10:46:59 -0700
Subject: [PATCH] tipc: Fix iov_len handling in message send path.

Use size_t to add together iov_len's from the iovec, error
out with -EMSGSIZE if total is greater than INT_MAX.

Reported-by: Dan Rosenberg <drosenberg@...curity.com>
Signed-off-by: David S. Miller <davem@...emloft.net>
---
 net/tipc/msg.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index ecb532f..b29eb8b 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -76,11 +76,15 @@ void tipc_msg_init(struct tipc_msg *m, u32 user, u32 type,
 
 int tipc_msg_calc_data_size(struct iovec const *msg_sect, u32 num_sect)
 {
-	int dsz = 0;
+	size_t dsz = 0;
 	int i;
 
 	for (i = 0; i < num_sect; i++)
 		dsz += msg_sect[i].iov_len;
+
+	if (dsz > INT_MAX)
+		return -EMSGSIZE;
+
 	return dsz;
 }
 
@@ -93,12 +97,15 @@ int tipc_msg_calc_data_size(struct iovec const *msg_sect, u32 num_sect)
  */
 
 int tipc_msg_build(struct tipc_msg *hdr,
-			    struct iovec const *msg_sect, u32 num_sect,
-			    int max_size, int usrmem, struct sk_buff** buf)
+		      struct iovec const *msg_sect, u32 num_sect,
+		      int max_size, int usrmem, struct sk_buff** buf)
 {
 	int dsz, sz, hsz, pos, res, cnt;
 
 	dsz = tipc_msg_calc_data_size(msg_sect, num_sect);
+	if (dsz < 0)
+		return dsz;
+
 	if (unlikely(dsz > TIPC_MAX_USER_MSG_SIZE)) {
 		*buf = NULL;
 		return -EINVAL;
-- 
1.7.3.2

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html