stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	Ilja Van Sprundel <ivansprundel@ioactive.com>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.4 23/23] sctp: validate from_addr_param return
Date: Fri, 24 Sep 2021 14:44:04 +0200	[thread overview]
Message-ID: <20210924124328.572238242@linuxfoundation.org> (raw)
In-Reply-To: <20210924124327.816210800@linuxfoundation.org>

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

commit 0c5dc070ff3d6246d22ddd931f23a6266249e3db upstream.

Ilja reported that, simply putting it, nothing was validating that
from_addr_param functions were operating on initialized memory. That is,
the parameter itself was being validated by sctp_walk_params, but it
doesn't check for types and their specific sizes and it could be a 0-length
one, causing from_addr_param to potentially work over the next parameter or
even uninitialized memory.

The fix here is to, in all calls to from_addr_param, check if enough space
is there for the wanted IP address type.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/net/sctp/structs.h |    2 +-
 net/sctp/bind_addr.c       |   20 +++++++++++---------
 net/sctp/input.c           |    6 ++++--
 net/sctp/ipv6.c            |    7 ++++++-
 net/sctp/protocol.c        |    7 ++++++-
 net/sctp/sm_make_chunk.c   |   29 ++++++++++++++++-------------
 6 files changed, 44 insertions(+), 27 deletions(-)

--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -469,7 +469,7 @@ struct sctp_af {
 					 int saddr);
 	void		(*from_sk)	(union sctp_addr *,
 					 struct sock *sk);
-	void		(*from_addr_param) (union sctp_addr *,
+	bool		(*from_addr_param) (union sctp_addr *,
 					    union sctp_addr_param *,
 					    __be16 port, int iif);
 	int		(*to_addr_param) (const union sctp_addr *,
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -284,19 +284,15 @@ int sctp_raw_to_bind_addrs(struct sctp_b
 		rawaddr = (union sctp_addr_param *)raw_addr_list;
 
 		af = sctp_get_af_specific(param_type2af(param->type));
-		if (unlikely(!af)) {
+		if (unlikely(!af) ||
+		    !af->from_addr_param(&addr, rawaddr, htons(port), 0)) {
 			retval = -EINVAL;
-			sctp_bind_addr_clean(bp);
-			break;
+			goto out_err;
 		}
 
-		af->from_addr_param(&addr, rawaddr, htons(port), 0);
 		retval = sctp_add_bind_addr(bp, &addr, SCTP_ADDR_SRC, gfp);
-		if (retval) {
-			/* Can't finish building the list, clean up. */
-			sctp_bind_addr_clean(bp);
-			break;
-		}
+		if (retval)
+			goto out_err;
 
 		len = ntohs(param->length);
 		addrs_len -= len;
@@ -304,6 +300,12 @@ int sctp_raw_to_bind_addrs(struct sctp_b
 	}
 
 	return retval;
+
+out_err:
+	if (retval)
+		sctp_bind_addr_clean(bp);
+
+	return retval;
 }
 
 /********************************************************************
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -972,7 +972,8 @@ static struct sctp_association *__sctp_r
 		if (!af)
 			continue;
 
-		af->from_addr_param(paddr, params.addr, sh->source, 0);
+		if (!af->from_addr_param(paddr, params.addr, sh->source, 0))
+			continue;
 
 		asoc = __sctp_lookup_association(net, laddr, paddr, &transport);
 		if (asoc)
@@ -1018,7 +1019,8 @@ static struct sctp_association *__sctp_r
 	if (unlikely(!af))
 		return NULL;
 
-	af->from_addr_param(&paddr, param, peer_port, 0);
+	if (af->from_addr_param(&paddr, param, peer_port, 0))
+		return NULL;
 
 	return __sctp_lookup_association(net, laddr, &paddr, transportp);
 }
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -488,15 +488,20 @@ static void sctp_v6_to_sk_daddr(union sc
 }
 
 /* Initialize a sctp_addr from an address parameter. */
-static void sctp_v6_from_addr_param(union sctp_addr *addr,
+static bool sctp_v6_from_addr_param(union sctp_addr *addr,
 				    union sctp_addr_param *param,
 				    __be16 port, int iif)
 {
+	if (ntohs(param->v6.param_hdr.length) < sizeof(struct sctp_ipv6addr_param))
+		return false;
+
 	addr->v6.sin6_family = AF_INET6;
 	addr->v6.sin6_port = port;
 	addr->v6.sin6_flowinfo = 0; /* BUG */
 	addr->v6.sin6_addr = param->v6.addr;
 	addr->v6.sin6_scope_id = iif;
+
+	return true;
 }
 
 /* Initialize an address parameter from a sctp_addr and return the length
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -272,14 +272,19 @@ static void sctp_v4_to_sk_daddr(union sc
 }
 
 /* Initialize a sctp_addr from an address parameter. */
-static void sctp_v4_from_addr_param(union sctp_addr *addr,
+static bool sctp_v4_from_addr_param(union sctp_addr *addr,
 				    union sctp_addr_param *param,
 				    __be16 port, int iif)
 {
+	if (ntohs(param->v4.param_hdr.length) < sizeof(struct sctp_ipv4addr_param))
+		return false;
+
 	addr->v4.sin_family = AF_INET;
 	addr->v4.sin_port = port;
 	addr->v4.sin_addr.s_addr = param->v4.addr.s_addr;
 	memset(addr->v4.sin_zero, 0, sizeof(addr->v4.sin_zero));
+
+	return true;
 }
 
 /* Initialize an address parameter from a sctp_addr and return the length
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2333,11 +2333,13 @@ int sctp_process_init(struct sctp_associ
 
 	/* Process the initialization parameters.  */
 	sctp_walk_params(param, peer_init, init_hdr.params) {
-		if (!src_match && (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
-		    param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
+		if (!src_match &&
+		    (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
+		     param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
 			af = sctp_get_af_specific(param_type2af(param.p->type));
-			af->from_addr_param(&addr, param.addr,
-					    chunk->sctp_hdr->source, 0);
+			if (!af->from_addr_param(&addr, param.addr,
+						 chunk->sctp_hdr->source, 0))
+				continue;
 			if (sctp_cmp_addr_exact(sctp_source(chunk), &addr))
 				src_match = 1;
 		}
@@ -2531,7 +2533,8 @@ static int sctp_process_param(struct sct
 			break;
 do_addr_param:
 		af = sctp_get_af_specific(param_type2af(param.p->type));
-		af->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0);
+		if (!af->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0))
+			break;
 		scope = sctp_scope(peer_addr);
 		if (sctp_in_scope(net, &addr, scope))
 			if (!sctp_assoc_add_peer(asoc, &addr, gfp, SCTP_UNCONFIRMED))
@@ -2624,15 +2627,13 @@ do_addr_param:
 		addr_param = param.v + sizeof(sctp_addip_param_t);
 
 		af = sctp_get_af_specific(param_type2af(addr_param->p.type));
-		if (af == NULL)
+		if (!af)
 			break;
 
-		af->from_addr_param(&addr, addr_param,
-				    htons(asoc->peer.port), 0);
+		if (!af->from_addr_param(&addr, addr_param,
+					 htons(asoc->peer.port), 0))
+			break;
 
-		/* if the address is invalid, we can't process it.
-		 * XXX: see spec for what to do.
-		 */
 		if (!af->addr_valid(&addr, NULL, NULL))
 			break;
 
@@ -3042,7 +3043,8 @@ static __be16 sctp_process_asconf_param(
 	if (unlikely(!af))
 		return SCTP_ERROR_DNS_FAILED;
 
-	af->from_addr_param(&addr, addr_param, htons(asoc->peer.port), 0);
+	if (!af->from_addr_param(&addr, addr_param, htons(asoc->peer.port), 0))
+		return SCTP_ERROR_DNS_FAILED;
 
 	/* ADDIP 4.2.1  This parameter MUST NOT contain a broadcast
 	 * or multicast address.
@@ -3308,7 +3310,8 @@ static void sctp_asconf_param_success(st
 
 	/* We have checked the packet before, so we do not check again.	*/
 	af = sctp_get_af_specific(param_type2af(addr_param->p.type));
-	af->from_addr_param(&addr, addr_param, htons(bp->port), 0);
+	if (!af->from_addr_param(&addr, addr_param, htons(bp->port), 0))
+		return;
 
 	switch (asconf_param->param_hdr.type) {
 	case SCTP_PARAM_ADD_IP:



  parent reply	other threads:[~2021-09-24 12:46 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24 12:43 [PATCH 4.4 00/23] 4.4.285-rc1 review Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 01/23] s390/bpf: Fix optimizing out zero-extensions Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 02/23] PM / wakeirq: Fix unbalanced IRQ enable for wakeirq Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 03/23] sctp: validate chunk size in __rcv_asconf_lookup Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 04/23] sctp: add param size validation for SCTP_PARAM_SET_PRIMARY Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 05/23] thermal/drivers/exynos: Fix an error code in exynos_tmu_probe() Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 06/23] 9p/trans_virtio: Remove sysfs file on probe failure Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 07/23] prctl: allow to setup brk for et_dyn executables Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 08/23] profiling: fix shift-out-of-bounds bugs Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 09/23] pwm: mxs: Dont modify HW state in .probe() after the PWM chip was registered Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 10/23] dmaengine: acpi-dma: check for 64-bit MMIO address Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 11/23] dmaengine: acpi: Avoid comparison GSI with Linux vIRQ Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 12/23] parisc: Move pci_dev_is_behind_card_dino to where it is used Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 13/23] dmaengine: ioat: depends on !UML Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 14/23] ceph: lockdep annotations for try_nonblocking_invalidate Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 15/23] nilfs2: fix memory leak in nilfs_sysfs_create_device_group Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 16/23] nilfs2: fix NULL pointer in nilfs_##name##_attr_release Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 17/23] nilfs2: fix memory leak in nilfs_sysfs_create_##name##_group Greg Kroah-Hartman
2021-09-24 12:43 ` [PATCH 4.4 18/23] nilfs2: fix memory leak in nilfs_sysfs_delete_##name##_group Greg Kroah-Hartman
2021-09-24 12:44 ` [PATCH 4.4 19/23] nilfs2: fix memory leak in nilfs_sysfs_create_snapshot_group Greg Kroah-Hartman
2021-09-24 12:44 ` [PATCH 4.4 20/23] nilfs2: fix memory leak in nilfs_sysfs_delete_snapshot_group Greg Kroah-Hartman
2021-09-24 12:44 ` [PATCH 4.4 21/23] blk-throttle: fix UAF by deleteing timer in blk_throtl_exit() Greg Kroah-Hartman
2021-09-24 12:44 ` [PATCH 4.4 22/23] drm/nouveau/nvkm: Replace -ENOSYS with -ENODEV Greg Kroah-Hartman
2021-09-24 12:44 ` Greg Kroah-Hartman [this message]
2021-09-24 13:50 ` [PATCH 4.4 00/23] 4.4.285-rc1 review Daniel Díaz
2021-09-25 11:45   ` Greg Kroah-Hartman
2021-09-24 17:50 ` Jon Hunter
2021-09-24 21:50 ` Pavel Machek
2021-09-24 21:55 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210924124328.572238242@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=ivansprundel@ioactive.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).