[PATCH][SECURITY] secmark: nul-terminate secdata

[PATCH][SECURITY] secmark: nul-terminate secdata

[James Morris]

The patch below fixes a problem in the iptables SECMARK target, where the 
user-supplied 'selctx' string may not be nul-terminated.

>From initial analysis, it seems that the strlen() called from 
selinux_string_to_sid() could run until it arbitrarily finds a zero, and 
possibly cause a kernel oops before then.

The impact of this appears limited because the operation requires 
CAP_NET_ADMIN, which is essentially always root.  Also, the module is not 
yet in wide use.

Please apply.

Note: some other iptables modules which handle strings supplied from 
userspace may require a similar fix (e.g. xt_string looks suspect at first 
glance).

cc'd Chris Wright, as this patch seems like a candidate for the stable 
tree.

Signed-off-by: James Morris <jmorris@namei.org>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>

---

diff -purN -X dontdiff linux-2.6.18-rc2-mm1.o/net/netfilter/xt_SECMARK.c linux-2.6.18-rc2-mm1.w/net/netfilter/xt_SECMARK.c
--- linux-2.6.18-rc2-mm1.o/net/netfilter/xt_SECMARK.c	2006-07-28 11:01:46.000000000 -0400
+++ linux-2.6.18-rc2-mm1.w/net/netfilter/xt_SECMARK.c	2006-07-28 11:15:44.000000000 -0400
@@ -57,6 +57,8 @@ static int checkentry_selinux(struct xt_
 {
 	int err;
 	struct xt_secmark_target_selinux_info *sel = &info->u.sel;
+	
+	sel->selctx[SECMARK_SELCTX_MAX - 1] = '\0';
 
 	err = selinux_string_to_sid(sel->selctx, &sel->selsid);
 	if (err) {

Re: [PATCH][SECURITY] secmark: nul-terminate secdata

[Patrick McHardy]

James Morris wrote:
> The patch below fixes a problem in the iptables SECMARK target, where the 
> user-supplied 'selctx' string may not be nul-terminated.
> 
>>From initial analysis, it seems that the strlen() called from 
> selinux_string_to_sid() could run until it arbitrarily finds a zero, and 
> possibly cause a kernel oops before then.
> 
> The impact of this appears limited because the operation requires 
> CAP_NET_ADMIN, which is essentially always root.  Also, the module is not 
> yet in wide use.
> 
> Please apply.
> 
> Note: some other iptables modules which handle strings supplied from 
> userspace may require a similar fix (e.g. xt_string looks suspect at first 
> glance).


I'll look into these, but it won't be the last of these problems. At the
last netfilter workshop Rusty spent some time figuring out "how many
iptables crashes (triggered by root) can I find in a few minutes", and
it was quite a lot. So far we've hoped for pkttables to make everything
better, but it looks like things like OpenVZ will beat us.

Re: [PATCH][SECURITY] secmark: nul-terminate secdata

[David Miller]

From: James Morris <jmorris@namei.org>
Date: Fri, 28 Jul 2006 17:00:15 -0400 (EDT)

> The patch below fixes a problem in the iptables SECMARK target, where the 
> user-supplied 'selctx' string may not be nul-terminated.

Applied, thanks James.

Re: [PATCH][SECURITY] secmark: nul-terminate secdata

[Chris Wright]

* James Morris (jmorris@namei.org) wrote:
> cc'd Chris Wright, as this patch seems like a candidate for the stable 
> tree.

Would be, but I thought secmark went in post 2.6.17.  And I expect Dave
will push this well before 2.6.18.

thanks,
-chris