[IPSEC] Fixed alg_key_len usage in attach_one_algo

[IPSEC] Fixed alg_key_len usage in attach_one_algo

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

Hi Dave:

This bug was discovered by a Linux box running in a remote corner
of Australia :)

The variable alg_key_len is in bits and not bytes.  The function
attach_one_algo is currently using it as if it were in bytes.
This causes it to read memory which may not be there.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-user-1 --]
[-- Type: text/plain, Size: 649 bytes --]

--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -162,6 +162,7 @@ static int attach_one_algo(struct xfrm_a
 	struct rtattr *rta = u_arg;
 	struct xfrm_algo *p, *ualg;
 	struct xfrm_algo_desc *algo;
+	int len;
 
 	if (!rta)
 		return 0;
@@ -173,11 +174,12 @@ static int attach_one_algo(struct xfrm_a
 		return -ENOSYS;
 	*props = algo->desc.sadb_alg_id;
 
-	p = kmalloc(sizeof(*ualg) + ualg->alg_key_len, GFP_KERNEL);
+	len = sizeof(*ualg) + (ualg->alg_key_len + 7U) / 8;
+	p = kmalloc(len, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 
-	memcpy(p, ualg, sizeof(*ualg) + ualg->alg_key_len);
+	memcpy(p, ualg, len);
 	*algpp = p;
 	return 0;
 }

[IPSEC] Verify key payload in verify_one_algo

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

Hi Dave:

We need to verify that the payload contains enough data so that
attach_one_algo can copy alg_key_len bits from the payload.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-user-2 --]
[-- Type: text/plain, Size: 533 bytes --]

--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -34,14 +34,21 @@ static int verify_one_alg(struct rtattr 
 {
 	struct rtattr *rt = xfrma[type - 1];
 	struct xfrm_algo *algp;
+	int len;
 
 	if (!rt)
 		return 0;
 
-	if ((rt->rta_len - sizeof(*rt)) < sizeof(*algp))
+	len = (rt->rta_len - sizeof(*rt)) - sizeof(*algp);
+	if (len < 0)
 		return -EINVAL;
 
 	algp = RTA_DATA(rt);
+
+	len -= (algp->alg_key_len + 7U) / 8; 
+	if (len < 0)
+		return -EINVAL;
+
 	switch (type) {
 	case XFRMA_ALG_AUTH:
 		if (!algp->alg_key_len &&

Re: [IPSEC] Verify key payload in verify_one_algo

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 19 May 2005 12:15:05 +1000

> We need to verify that the payload contains enough data so that
> attach_one_algo can copy alg_key_len bits from the payload.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Also applied, thanks again.

I gather from these two patches that once you found the
first problem you went around auditing alg_key_len usage
and these were the only two bugs you spotted.  Right?

Re: [IPSEC] Verify key payload in verify_one_algo

[Herbert Xu]

On Thu, May 19, 2005 at 12:40:59PM -0700, David S. Miller wrote:
> 
> I gather from these two patches that once you found the
> first problem you went around auditing alg_key_len usage
> and these were the only two bugs you spotted.  Right?

Correct.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [IPSEC] Fixed alg_key_len usage in attach_one_algo

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 19 May 2005 12:03:02 +1000

> This bug was discovered by a Linux box running in a remote corner
> of Australia :)

Hehe :-)

> The variable alg_key_len is in bits and not bytes.  The function
> attach_one_algo is currently using it as if it were in bytes.
> This causes it to read memory which may not be there.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, thanks Herbert.