Commit 9fb6b81e authored by Pablo Neira Ayuso's avatar Pablo Neira Ayuso Committed by Greg Kroah-Hartman

netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code

commit 4c559f15 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: f09943fe ("[NETFILTER]: nf_conntrack/nf_nat: add PPTP helper port")
Reported-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent e70fb3ef
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
#include <linux/netfilter/nf_conntrack_common.h> #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 */ /* state of the control session */
enum pptp_ctrlsess_state { enum pptp_ctrlsess_state {
......
...@@ -165,8 +165,7 @@ pptp_outbound_pkt(struct sk_buff *skb, ...@@ -165,8 +165,7 @@ pptp_outbound_pkt(struct sk_buff *skb,
break; break;
default: default:
pr_debug("unknown outbound packet 0x%04x:%s\n", msg, pr_debug("unknown outbound packet 0x%04x:%s\n", msg,
msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] : pptp_msg_name(msg));
pptp_msg_name[0]);
/* fall through */ /* fall through */
case PPTP_SET_LINK_INFO: case PPTP_SET_LINK_INFO:
/* only need to NAT in case PAC is behind NAT box */ /* only need to NAT in case PAC is behind NAT box */
...@@ -267,9 +266,7 @@ pptp_inbound_pkt(struct sk_buff *skb, ...@@ -267,9 +266,7 @@ pptp_inbound_pkt(struct sk_buff *skb,
pcid_off = offsetof(union pptp_ctrl_union, setlink.peersCallID); pcid_off = offsetof(union pptp_ctrl_union, setlink.peersCallID);
break; break;
default: default:
pr_debug("unknown inbound packet %s\n", pr_debug("unknown inbound packet %s\n", pptp_msg_name(msg));
msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] :
pptp_msg_name[0]);
/* fall through */ /* fall through */
case PPTP_START_SESSION_REQUEST: case PPTP_START_SESSION_REQUEST:
case PPTP_START_SESSION_REPLY: case PPTP_START_SESSION_REPLY:
......
...@@ -71,24 +71,32 @@ EXPORT_SYMBOL_GPL(nf_nat_pptp_hook_expectfn); ...@@ -71,24 +71,32 @@ EXPORT_SYMBOL_GPL(nf_nat_pptp_hook_expectfn);
#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) #if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
/* PptpControlMessageType names */ /* PptpControlMessageType names */
const char *const pptp_msg_name[] = { static const char *const pptp_msg_name_array[PPTP_MSG_MAX + 1] = {
"UNKNOWN_MESSAGE", [0] = "UNKNOWN_MESSAGE",
"START_SESSION_REQUEST", [PPTP_START_SESSION_REQUEST] = "START_SESSION_REQUEST",
"START_SESSION_REPLY", [PPTP_START_SESSION_REPLY] = "START_SESSION_REPLY",
"STOP_SESSION_REQUEST", [PPTP_STOP_SESSION_REQUEST] = "STOP_SESSION_REQUEST",
"STOP_SESSION_REPLY", [PPTP_STOP_SESSION_REPLY] = "STOP_SESSION_REPLY",
"ECHO_REQUEST", [PPTP_ECHO_REQUEST] = "ECHO_REQUEST",
"ECHO_REPLY", [PPTP_ECHO_REPLY] = "ECHO_REPLY",
"OUT_CALL_REQUEST", [PPTP_OUT_CALL_REQUEST] = "OUT_CALL_REQUEST",
"OUT_CALL_REPLY", [PPTP_OUT_CALL_REPLY] = "OUT_CALL_REPLY",
"IN_CALL_REQUEST", [PPTP_IN_CALL_REQUEST] = "IN_CALL_REQUEST",
"IN_CALL_REPLY", [PPTP_IN_CALL_REPLY] = "IN_CALL_REPLY",
"IN_CALL_CONNECT", [PPTP_IN_CALL_CONNECT] = "IN_CALL_CONNECT",
"CALL_CLEAR_REQUEST", [PPTP_CALL_CLEAR_REQUEST] = "CALL_CLEAR_REQUEST",
"CALL_DISCONNECT_NOTIFY", [PPTP_CALL_DISCONNECT_NOTIFY] = "CALL_DISCONNECT_NOTIFY",
"WAN_ERROR_NOTIFY", [PPTP_WAN_ERROR_NOTIFY] = "WAN_ERROR_NOTIFY",
"SET_LINK_INFO" [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); EXPORT_SYMBOL(pptp_msg_name);
#endif #endif
...@@ -275,7 +283,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -275,7 +283,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
typeof(nf_nat_pptp_hook_inbound) nf_nat_pptp_inbound; typeof(nf_nat_pptp_hook_inbound) nf_nat_pptp_inbound;
msg = ntohs(ctlh->messageType); 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) { switch (msg) {
case PPTP_START_SESSION_REPLY: case PPTP_START_SESSION_REPLY:
...@@ -310,7 +318,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -310,7 +318,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
pcid = pptpReq->ocack.peersCallID; pcid = pptpReq->ocack.peersCallID;
if (info->pns_call_id != pcid) if (info->pns_call_id != pcid)
goto invalid; 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)); ntohs(cid), ntohs(pcid));
if (pptpReq->ocack.resultCode == PPTP_OUTCALL_CONNECT) { if (pptpReq->ocack.resultCode == PPTP_OUTCALL_CONNECT) {
...@@ -327,7 +335,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -327,7 +335,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
goto invalid; goto invalid;
cid = pptpReq->icreq.callID; 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->cstate = PPTP_CALL_IN_REQ;
info->pac_call_id = cid; info->pac_call_id = cid;
break; break;
...@@ -346,7 +354,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -346,7 +354,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
if (info->pns_call_id != pcid) if (info->pns_call_id != pcid)
goto invalid; 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; info->cstate = PPTP_CALL_IN_CONF;
/* we expect a GRE connection from PAC to PNS */ /* we expect a GRE connection from PAC to PNS */
...@@ -356,7 +364,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -356,7 +364,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
case PPTP_CALL_DISCONNECT_NOTIFY: case PPTP_CALL_DISCONNECT_NOTIFY:
/* server confirms disconnect */ /* server confirms disconnect */
cid = pptpReq->disc.callID; 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; info->cstate = PPTP_CALL_NONE;
/* untrack this call id, unexpect GRE packets */ /* untrack this call id, unexpect GRE packets */
...@@ -383,7 +391,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -383,7 +391,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
invalid: invalid:
pr_debug("invalid %s: type=%d cid=%u pcid=%u " pr_debug("invalid %s: type=%d cid=%u pcid=%u "
"cstate=%d sstate=%d pns_cid=%u pac_cid=%u\n", "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, msg, ntohs(cid), ntohs(pcid), info->cstate, info->sstate,
ntohs(info->pns_call_id), ntohs(info->pac_call_id)); ntohs(info->pns_call_id), ntohs(info->pac_call_id));
return NF_ACCEPT; return NF_ACCEPT;
...@@ -403,7 +411,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -403,7 +411,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
typeof(nf_nat_pptp_hook_outbound) nf_nat_pptp_outbound; typeof(nf_nat_pptp_hook_outbound) nf_nat_pptp_outbound;
msg = ntohs(ctlh->messageType); 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) { switch (msg) {
case PPTP_START_SESSION_REQUEST: case PPTP_START_SESSION_REQUEST:
...@@ -425,7 +433,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -425,7 +433,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
info->cstate = PPTP_CALL_OUT_REQ; info->cstate = PPTP_CALL_OUT_REQ;
/* track PNS call id */ /* track PNS call id */
cid = pptpReq->ocreq.callID; 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; info->pns_call_id = cid;
break; break;
...@@ -439,7 +447,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -439,7 +447,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
pcid = pptpReq->icack.peersCallID; pcid = pptpReq->icack.peersCallID;
if (info->pac_call_id != pcid) if (info->pac_call_id != pcid)
goto invalid; 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)); ntohs(cid), ntohs(pcid));
if (pptpReq->icack.resultCode == PPTP_INCALL_ACCEPT) { if (pptpReq->icack.resultCode == PPTP_INCALL_ACCEPT) {
...@@ -479,7 +487,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff, ...@@ -479,7 +487,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
invalid: invalid:
pr_debug("invalid %s: type=%d cid=%u pcid=%u " pr_debug("invalid %s: type=%d cid=%u pcid=%u "
"cstate=%d sstate=%d pns_cid=%u pac_cid=%u\n", "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, msg, ntohs(cid), ntohs(pcid), info->cstate, info->sstate,
ntohs(info->pns_call_id), ntohs(info->pac_call_id)); ntohs(info->pns_call_id), ntohs(info->pac_call_id));
return NF_ACCEPT; return NF_ACCEPT;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment