nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code

Browse source 
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: f09943fefe ("[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>
This commit is contained in:
Pablo Neira Ayuso 2020-05-14 14:14:23 +02:00
parent a164b95ad6
commit 4c559f15ef
3 changed files with 38 additions and 33 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

2
include/linux/netfilter/nf_conntrack_pptp.h
View file

@ -10,7 +10,7 @@
#include <net/netfilter/nf_conntrack_expect.h> #include <net/netfilter/nf_conntrack_expect.h>
#include <uapi/linux/netfilter/nf_conntrack_tuple_common.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 */ /* state of the control session */
enum pptp_ctrlsess_state { enum pptp_ctrlsess_state {

7
net/ipv4/netfilter/nf_nat_pptp.c
View file

@ -166,8 +166,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]);
fallthrough; fallthrough;
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 */
@ -268,9 +267,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]);
fallthrough; fallthrough;
case PPTP_START_SESSION_REQUEST: case PPTP_START_SESSION_REQUEST:
case PPTP_START_SESSION_REPLY: case PPTP_START_SESSION_REPLY:

62
net/netfilter/nf_conntrack_pptp.c
View file

@ -72,24 +72,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
@ -276,7 +284,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:
@ -311,7 +319,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) {
@ -328,7 +336,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;
@ -347,7 +355,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 */
@ -357,7 +365,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 */
@ -384,7 +392,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;
@ -404,7 +412,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:
@ -426,7 +434,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;
@ -440,7 +448,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) {
@ -480,7 +488,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;