Skip to contentSkip to conversation

sctp: additional overflow fixes

PATCH Applied Vlad Yasevich 3 participants 7 messages Aug 27, 2008 – Aug 28, 2008 1 message not in the archive

Subsystems (3)
Networking Network drivers SCTP protocol
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Vlad Yasevich

author
#1 lore raw

cover sctp: additional overflow fixes

Hi David

Thanks to Eugene Teo, the following is two more patches that finish up the overflow fixed in SCTP AUTH api.

Thanks
-vlad

author
#2 lore raw

PATCH 1/2 sctp: correct bounds check in sctp_setsockopt_auth_key

In reply to a message not in the archive

The bonds check to prevent buffer overlflow was not exactly right. It still allowed overflow of up to 8 bytes which is sizeof(struct sctp_authkey).

Since optlen is already checked against the size of that struct, we are guaranteed not to cause interger overflow either.

Signed-off-by Vlad Yasevich <[email protected]>
---
 net/sctp/socket.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/sctp/socket.c +1 -1
index afa952e..9b9b2c3 100644
@@ -3144,7 +3144,7 @@ static int sctp_setsockopt_auth_key(struct sock *sk,
 		goto out;
 	}
 
-	if (authkey->sca_keylength > optlen) {
+	if (authkey->sca_keylength > optlen - sizeof(struct sctp_authkey)) {
 		ret = -EINVAL;
 		goto out;
 	}
Signature · 1 line
1.5.2.5
maintainer Applied
#3 lore raw

Re: [PATCH 1/2] sctp: correct bounds check in sctp_setsockopt_auth_key

From: Vlad Yasevich <[email protected]>
Date: Wed, 27 Aug 2008 16:03:34 -0400

Quoted 8 lines

The bonds check to prevent buffer overlflow was not exactly right. It still allowed overflow of up to 8 bytes which is sizeof(struct sctp_authkey).

Since optlen is already checked against the size of that struct, we are guaranteed not to cause interger overflow either.

Signed-off-by Vlad Yasevich <[email protected]>

Applied, and queued for -stable.

Acked-by on v1 1/2
#4 lore raw

Re: [PATCH 1/2] sctp: correct bounds check in sctp_setsockopt_auth_key

In reply to Vlad Yasevich
Quoted 8 lines · Vlad Yasevich

The bonds check to prevent buffer overlflow was not exactly right. It still allowed overflow of up to 8 bytes which is sizeof(struct sctp_authkey).

Since optlen is already checked against the size of that struct, we are guaranteed not to cause interger overflow either.

Signed-off-by Vlad Yasevich <[email protected]>
Acked-by Eugene Teo <[email protected]>

Eugene

author
#5 lore raw

PATCH 2/2 sctp: fix random memory dereference with SCTP_HMAC_IDENT option.

In reply to a message not in the archive

The number of identifiers needs to be checked against the option length. Also, the identifier index provided needs to be verified to make sure that it doesn't exceed the bounds of the array.

Signed-off-by Vlad Yasevich <[email protected]>
---
 net/sctp/auth.c   |    3 +++
 net/sctp/socket.c |    6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)
net/sctp/auth.c +3 -0
index 1fcb4cf..52db5f6 100644
@@ -786,6 +786,9 @@ int sctp_auth_ep_set_hmacs(struct sctp_endpoint *ep,
 	for (i = 0; i < hmacs->shmac_num_idents; i++) {
 		id = hmacs->shmac_idents[i];
 
+		if (id > SCTP_AUTH_HMAC_ID_MAX)
+			return -EOPNOTSUPP;
+
 		if (SCTP_AUTH_HMAC_ID_SHA1 = id)
 			has_sha1 = 1;
net/sctp/socket.c +4 -2
index 9b9b2c3..5ffb9de 100644
@@ -3086,6 +3086,7 @@ static int sctp_setsockopt_hmac_ident(struct sock *sk,
 				    int optlen)
 {
 	struct sctp_hmacalgo *hmacs;
+	u32 idents;
 	int err;
 
 	if (!sctp_auth_enable)

@@ -3103,8 +3104,9 @@ static int sctp_setsockopt_hmac_ident(struct sock *sk,
 		goto out;
 	}
 
-	if (hmacs->shmac_num_idents = 0 ||
-	    hmacs->shmac_num_idents > SCTP_AUTH_NUM_HMACS) {
+	idents = hmacs->shmac_num_idents;
+	if (idents = 0 || idents > SCTP_AUTH_NUM_HMACS ||
+	    (idents * sizeof(u16)) > (optlen - sizeof(struct sctp_hmacalgo))) {
 		err = -EINVAL;
 		goto out;
 	}
Signature · 1 line
1.5.2.5
maintainer
#6 lore raw

Re: [PATCH 2/2] sctp: fix random memory dereference with SCTP_HMAC_IDENT option.

From: Vlad Yasevich <[email protected]>
Date: Wed, 27 Aug 2008 16:03:35 -0400

Quoted 5 lines

The number of identifiers needs to be checked against the option length. Also, the identifier index provided needs to be verified to make sure that it doesn't exceed the bounds of the array.

Signed-off-by Vlad Yasevich <[email protected]>

Also applied and queued to -stable, thanks Vlad.