linux-kernel.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, Dan Carpenter <dan.carpenter@oracle.com>,
	Pablo Neira Ayuso <pablo@netfilter.org>
Subject: [PATCH 4.9 52/61] netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code
Date: Mon,  1 Jun 2020 19:53:59 +0200	[thread overview]
Message-ID: <20200601174021.117625302@linuxfoundation.org> (raw)
In-Reply-To: <20200601174010.316778377@linuxfoundation.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>

commit 4c559f15efcc43b996f4da528cd7f9483aaca36d upstream.

Dan Carpenter says: "Smatch complains that the value for "cmd" comes
from the network and can't be trusted."

Add pptp_msg_name() helper function that checks for the array boundary.

Fixes: f09943fefe6b ("[NETFILTER]: nf_conntrack/nf_nat: add PPTP helper port")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 include/linux/netfilter/nf_conntrack_pptp.h |    2 
 net/ipv4/netfilter/nf_nat_pptp.c            |    7 ---
 net/netfilter/nf_conntrack_pptp.c           |   62 +++++++++++++++-------------
 3 files changed, 38 insertions(+), 33 deletions(-)

--- a/include/linux/netfilter/nf_conntrack_pptp.h
+++ b/include/linux/netfilter/nf_conntrack_pptp.h
@@ -4,7 +4,7 @@
 
 #include <linux/netfilter/nf_conntrack_common.h>
 
-extern const char *const pptp_msg_name[];
+extern const char *const pptp_msg_name(u_int16_t msg);
 
 /* state of the control session */
 enum pptp_ctrlsess_state {
--- a/net/ipv4/netfilter/nf_nat_pptp.c
+++ b/net/ipv4/netfilter/nf_nat_pptp.c
@@ -156,8 +156,7 @@ pptp_outbound_pkt(struct sk_buff *skb,
 		break;
 	default:
 		pr_debug("unknown outbound packet 0x%04x:%s\n", msg,
-			 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] :
-					       pptp_msg_name[0]);
+			 pptp_msg_name(msg));
 		/* fall through */
 	case PPTP_SET_LINK_INFO:
 		/* only need to NAT in case PAC is behind NAT box */
@@ -250,9 +249,7 @@ pptp_inbound_pkt(struct sk_buff *skb,
 		pcid_off = offsetof(union pptp_ctrl_union, setlink.peersCallID);
 		break;
 	default:
-		pr_debug("unknown inbound packet %s\n",
-			 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] :
-					       pptp_msg_name[0]);
+		pr_debug("unknown inbound packet %s\n", pptp_msg_name(msg));
 		/* fall through */
 	case PPTP_START_SESSION_REQUEST:
 	case PPTP_START_SESSION_REPLY:
--- a/net/netfilter/nf_conntrack_pptp.c
+++ b/net/netfilter/nf_conntrack_pptp.c
@@ -71,24 +71,32 @@ EXPORT_SYMBOL_GPL(nf_nat_pptp_hook_expec
 
 #if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
 /* PptpControlMessageType names */
-const char *const pptp_msg_name[] = {
-	"UNKNOWN_MESSAGE",
-	"START_SESSION_REQUEST",
-	"START_SESSION_REPLY",
-	"STOP_SESSION_REQUEST",
-	"STOP_SESSION_REPLY",
-	"ECHO_REQUEST",
-	"ECHO_REPLY",
-	"OUT_CALL_REQUEST",
-	"OUT_CALL_REPLY",
-	"IN_CALL_REQUEST",
-	"IN_CALL_REPLY",
-	"IN_CALL_CONNECT",
-	"CALL_CLEAR_REQUEST",
-	"CALL_DISCONNECT_NOTIFY",
-	"WAN_ERROR_NOTIFY",
-	"SET_LINK_INFO"
+static const char *const pptp_msg_name_array[PPTP_MSG_MAX + 1] = {
+	[0]				= "UNKNOWN_MESSAGE",
+	[PPTP_START_SESSION_REQUEST]	= "START_SESSION_REQUEST",
+	[PPTP_START_SESSION_REPLY]	= "START_SESSION_REPLY",
+	[PPTP_STOP_SESSION_REQUEST]	= "STOP_SESSION_REQUEST",
+	[PPTP_STOP_SESSION_REPLY]	= "STOP_SESSION_REPLY",
+	[PPTP_ECHO_REQUEST]		= "ECHO_REQUEST",
+	[PPTP_ECHO_REPLY]		= "ECHO_REPLY",
+	[PPTP_OUT_CALL_REQUEST]		= "OUT_CALL_REQUEST",
+	[PPTP_OUT_CALL_REPLY]		= "OUT_CALL_REPLY",
+	[PPTP_IN_CALL_REQUEST]		= "IN_CALL_REQUEST",
+	[PPTP_IN_CALL_REPLY]		= "IN_CALL_REPLY",
+	[PPTP_IN_CALL_CONNECT]		= "IN_CALL_CONNECT",
+	[PPTP_CALL_CLEAR_REQUEST]	= "CALL_CLEAR_REQUEST",
+	[PPTP_CALL_DISCONNECT_NOTIFY]	= "CALL_DISCONNECT_NOTIFY",
+	[PPTP_WAN_ERROR_NOTIFY]		= "WAN_ERROR_NOTIFY",
+	[PPTP_SET_LINK_INFO]		= "SET_LINK_INFO"
 };
+
+const char *const pptp_msg_name(u_int16_t msg)
+{
+	if (msg > PPTP_MSG_MAX)
+		return pptp_msg_name_array[0];
+
+	return pptp_msg_name_array[msg];
+}
 EXPORT_SYMBOL(pptp_msg_name);
 #endif
 
@@ -277,7 +285,7 @@ pptp_inbound_pkt(struct sk_buff *skb, un
 	typeof(nf_nat_pptp_hook_inbound) nf_nat_pptp_inbound;
 
 	msg = ntohs(ctlh->messageType);
-	pr_debug("inbound control message %s\n", pptp_msg_name[msg]);
+	pr_debug("inbound control message %s\n", pptp_msg_name(msg));
 
 	switch (msg) {
 	case PPTP_START_SESSION_REPLY:
@@ -312,7 +320,7 @@ pptp_inbound_pkt(struct sk_buff *skb, un
 		pcid = pptpReq->ocack.peersCallID;
 		if (info->pns_call_id != pcid)
 			goto invalid;
-		pr_debug("%s, CID=%X, PCID=%X\n", pptp_msg_name[msg],
+		pr_debug("%s, CID=%X, PCID=%X\n", pptp_msg_name(msg),
 			 ntohs(cid), ntohs(pcid));
 
 		if (pptpReq->ocack.resultCode == PPTP_OUTCALL_CONNECT) {
@@ -329,7 +337,7 @@ pptp_inbound_pkt(struct sk_buff *skb, un
 			goto invalid;
 
 		cid = pptpReq->icreq.callID;
-		pr_debug("%s, CID=%X\n", pptp_msg_name[msg], ntohs(cid));
+		pr_debug("%s, CID=%X\n", pptp_msg_name(msg), ntohs(cid));
 		info->cstate = PPTP_CALL_IN_REQ;
 		info->pac_call_id = cid;
 		break;
@@ -348,7 +356,7 @@ pptp_inbound_pkt(struct sk_buff *skb, un
 		if (info->pns_call_id != pcid)
 			goto invalid;
 
-		pr_debug("%s, PCID=%X\n", pptp_msg_name[msg], ntohs(pcid));
+		pr_debug("%s, PCID=%X\n", pptp_msg_name(msg), ntohs(pcid));
 		info->cstate = PPTP_CALL_IN_CONF;
 
 		/* we expect a GRE connection from PAC to PNS */
@@ -358,7 +366,7 @@ pptp_inbound_pkt(struct sk_buff *skb, un
 	case PPTP_CALL_DISCONNECT_NOTIFY:
 		/* server confirms disconnect */
 		cid = pptpReq->disc.callID;
-		pr_debug("%s, CID=%X\n", pptp_msg_name[msg], ntohs(cid));
+		pr_debug("%s, CID=%X\n", pptp_msg_name(msg), ntohs(cid));
 		info->cstate = PPTP_CALL_NONE;
 
 		/* untrack this call id, unexpect GRE packets */
@@ -385,7 +393,7 @@ pptp_inbound_pkt(struct sk_buff *skb, un
 invalid:
 	pr_debug("invalid %s: type=%d cid=%u pcid=%u "
 		 "cstate=%d sstate=%d pns_cid=%u pac_cid=%u\n",
-		 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] : pptp_msg_name[0],
+		 pptp_msg_name(msg),
 		 msg, ntohs(cid), ntohs(pcid),  info->cstate, info->sstate,
 		 ntohs(info->pns_call_id), ntohs(info->pac_call_id));
 	return NF_ACCEPT;
@@ -405,7 +413,7 @@ pptp_outbound_pkt(struct sk_buff *skb, u
 	typeof(nf_nat_pptp_hook_outbound) nf_nat_pptp_outbound;
 
 	msg = ntohs(ctlh->messageType);
-	pr_debug("outbound control message %s\n", pptp_msg_name[msg]);
+	pr_debug("outbound control message %s\n", pptp_msg_name(msg));
 
 	switch (msg) {
 	case PPTP_START_SESSION_REQUEST:
@@ -427,7 +435,7 @@ pptp_outbound_pkt(struct sk_buff *skb, u
 		info->cstate = PPTP_CALL_OUT_REQ;
 		/* track PNS call id */
 		cid = pptpReq->ocreq.callID;
-		pr_debug("%s, CID=%X\n", pptp_msg_name[msg], ntohs(cid));
+		pr_debug("%s, CID=%X\n", pptp_msg_name(msg), ntohs(cid));
 		info->pns_call_id = cid;
 		break;
 
@@ -441,7 +449,7 @@ pptp_outbound_pkt(struct sk_buff *skb, u
 		pcid = pptpReq->icack.peersCallID;
 		if (info->pac_call_id != pcid)
 			goto invalid;
-		pr_debug("%s, CID=%X PCID=%X\n", pptp_msg_name[msg],
+		pr_debug("%s, CID=%X PCID=%X\n", pptp_msg_name(msg),
 			 ntohs(cid), ntohs(pcid));
 
 		if (pptpReq->icack.resultCode == PPTP_INCALL_ACCEPT) {
@@ -481,7 +489,7 @@ pptp_outbound_pkt(struct sk_buff *skb, u
 invalid:
 	pr_debug("invalid %s: type=%d cid=%u pcid=%u "
 		 "cstate=%d sstate=%d pns_cid=%u pac_cid=%u\n",
-		 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] : pptp_msg_name[0],
+		 pptp_msg_name(msg),
 		 msg, ntohs(cid), ntohs(pcid),  info->cstate, info->sstate,
 		 ntohs(info->pns_call_id), ntohs(info->pac_call_id));
 	return NF_ACCEPT;



  parent reply	other threads:[~2020-06-01 18:00 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-01 17:53 [PATCH 4.9 00/61] 4.9.226-rc1 review Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 01/61] ax25: fix setsockopt(SO_BINDTODEVICE) Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 02/61] net: ipip: fix wrong address family in init error path Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 03/61] net: revert "net: get rid of an signed integer overflow in ip_idents_reserve()" Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 04/61] net sched: fix reporting the first-time use timestamp Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 05/61] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 06/61] net/mlx5e: Update netdev txq on completions during closure Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 07/61] net: qrtr: Fix passing invalid reference to qrtr_local_enqueue() Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 08/61] net/mlx5: Add command entry handling completion Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 09/61] net: sun: fix missing release regions in cas_init_one() Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 10/61] net/mlx4_core: fix a memory leak bug Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 11/61] uapi: fix linux/if_pppol2tp.h userspace compilation errors Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 12/61] IB/cma: Fix reference count leak when no ipv4 addresses are set Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 13/61] gpio: tegra: mask GPIO IRQs during IRQ shutdown Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 14/61] net: microchip: encx24j600: add missed kthread_stop Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 15/61] gfs2: move privileged user check to gfs2_quota_lock_check Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 16/61] gfs2: dont call quota_unhold if quotas are not locked Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 17/61] cachefiles: Fix race between read_waiter and read_copier involving op->to_do Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 18/61] usb: gadget: legacy: fix redundant initialization warnings Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 19/61] cifs: Fix null pointer check in cifs_read Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 20/61] Input: usbtouchscreen - add support for BonXeon TP Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 21/61] Input: i8042 - add ThinkPad S230u to i8042 nomux list Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 22/61] Input: evdev - call input_flush_device() on release(), not flush() Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 23/61] Input: xpad - add custom init packet for Xbox One S controllers Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 24/61] Input: i8042 - add ThinkPad S230u to i8042 reset list Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 25/61] Input: synaptics-rmi4 - fix error return code in rmi_driver_probe() Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 26/61] ARM: 8843/1: use unified assembler in headers Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 27/61] ARM: uaccess: consolidate uaccess asm to asm/uaccess-asm.h Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 28/61] ARM: uaccess: integrate uaccess_save and uaccess_restore Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 29/61] ARM: uaccess: fix DACR mismatch with nested exceptions Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 30/61] IB/qib: Call kobject_put() when kobject_init_and_add() fails Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 31/61] ARM: dts: imx: Correct B850v3 clock assignment Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 32/61] ARM: dts: imx6q-bx50v3: Add internal switch Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 33/61] ARM: dts/imx6q-bx50v3: Set display interface clock parents Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 34/61] ALSA: hwdep: fix a left shifting 1 by 31 UB bug Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 35/61] ALSA: usb-audio: mixer: volume quirk for ESS Technology Asus USB DAC Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 36/61] exec: Always set cap_ambient in cap_bprm_set_creds Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 37/61] libceph: ignore pool overlay and cache logic on redirects Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 38/61] mm: remove VM_BUG_ON(PageSlab()) from page_mapcount() Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 39/61] fs/binfmt_elf.c: allocate initialized memory in fill_thread_core_info() Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 40/61] include/asm-generic/topology.h: guard cpumask_of_node() macro argument Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 41/61] iommu: Fix reference count leak in iommu_group_alloc Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 42/61] parisc: Fix kernel panic in mem_init() Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 43/61] mac80211: mesh: fix discovery timer re-arming issue / crash Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 44/61] x86/dma: Fix max PFN arithmetic overflow on 32 bit systems Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 45/61] xfrm: allow to accept packets with ipv6 NEXTHDR_HOP in xfrm_input Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 46/61] xfrm: fix a warning in xfrm_policy_insert_list Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 47/61] xfrm: fix a NULL-ptr deref in xfrm_local_error Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 48/61] vti4: eliminated some duplicate code Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 49/61] ip_vti: receive ipip packet by calling ip_tunnel_rcv Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 50/61] netfilter: nft_reject_bridge: enable reject with bridge vlan Greg Kroah-Hartman
2020-06-01 17:53 ` [PATCH 4.9 51/61] netfilter: ipset: Fix subcounter update skip Greg Kroah-Hartman
2020-06-01 17:53 ` Greg Kroah-Hartman [this message]
2020-06-01 17:54 ` [PATCH 4.9 53/61] qlcnic: fix missing release in qlcnic_83xx_interrupt_test Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 54/61] bonding: Fix reference count leak in bond_sysfs_slave_add Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 55/61] Revert "Input: i8042 - add ThinkPad S230u to i8042 nomux list" Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 56/61] netfilter: nf_conntrack_pptp: fix compilation warning with W=1 build Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 57/61] genirq/generic_pending: Do not lose pending affinity update Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 58/61] net: rtnl_configure_link: fix dev flags changes arg to __dev_notify_flags Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 59/61] mm/vmalloc.c: dont dereference possible NULL pointer in __vunmap() Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 60/61] sc16is7xx: move label err_spi to correct section Greg Kroah-Hartman
2020-06-01 17:54 ` [PATCH 4.9 61/61] net: hns: Fixes the missing put_device in positive leg for roce reset Greg Kroah-Hartman
2020-06-02  2:34 ` [PATCH 4.9 00/61] 4.9.226-rc1 review Guenter Roeck

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=20200601174021.117625302@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dan.carpenter@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --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).