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 <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 {

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

@ -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:

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)
/* 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;