netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code

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>

diff --git a/include/linux/netfilter/nf_conntrack_pptp.h b/include/linux/netfilter/nf_conntrack_pptp.h
index fcc409de31a406af2c8418e29077f0f0abe68699..6a4ff6d5ebc290801d2325c7fe4b4406383d52d7 100644 (file)
--- a/include/linux/netfilter/nf_conntrack_pptp.h
+++ b/include/linux/netfilter/nf_conntrack_pptp.h
@@ -10,7 +10,7 @@
 #include <net/netfilter/nf_conntrack_expect.h>
 #include <uapi/linux/netfilter/nf_conntrack_tuple_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 {

diff --git a/net/ipv4/netfilter/nf_nat_pptp.c b/net/ipv4/netfilter/nf_nat_pptp.c
index 3c25a467b3efc9c408ca6f51f622675c81ee4710..7afde8828b4c9c3f17f6ae3beea8d7f7d0b677eb 100644 (file)
--- a/net/ipv4/netfilter/nf_nat_pptp.c
+++ b/net/ipv4/netfilter/nf_nat_pptp.c
@@ -166,8 +166,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));
                fallthrough;
        case PPTP_SET_LINK_INFO:
                /* only need to NAT in case PAC is behind NAT box */
@@ -268,9 +267,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));
                fallthrough;
        case PPTP_START_SESSION_REQUEST:
        case PPTP_START_SESSION_REPLY:

diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c
index a971183f11af77ac1533de77a7778adab37d4758..7ad247784cfaa4b1f9c103b77337bb99857b55f1 100644 (file)
--- a/net/netfilter/nf_conntrack_pptp.c
+++ b/net/netfilter/nf_conntrack_pptp.c
@@ -72,24 +72,32 @@ EXPORT_SYMBOL_GPL(nf_nat_pptp_hook_expectfn);
 
 #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
 
@@ -276,7 +284,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
        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:
@@ -311,7 +319,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
                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) {
@@ -328,7 +336,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
                        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;
@@ -347,7 +355,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
                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 */
@@ -357,7 +365,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
        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 */
@@ -384,7 +392,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
 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;
@@ -404,7 +412,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
        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:
@@ -426,7 +434,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
                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;
 
@@ -440,7 +448,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
                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) {
@@ -480,7 +488,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
 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;