Skip to content

Commit 6a2b724

Browse files
winminummakynes
authored andcommitted
netfilter: nf_conntrack_sip: fix use of uninitialized rtp_addr in process_sdp
process_sdp() declares union nf_inet_addr rtp_addr on the stack and passes it to the nf_nat_sip sdp_session hook after walking the SDP media descriptions. However rtp_addr is only initialized inside the media loop when a recognized media type with a non-zero port is found. If the SDP body contains no m= lines, only inactive media sections (m=audio 0 ...) or only unrecognized media types, rtp_addr is never assigned. Despite that, the function still calls hooks->sdp_session() with &rtp_addr, causing nf_nat_sdp_session() to format the stale stack value as an IP address and rewrite the SDP session owner and connection lines with it. With CONFIG_INIT_STACK_ALL_ZERO (default on most distributions) this results in the session-level o= and c= addresses being rewritten to 0.0.0.0 for inactive SDP sessions. Without stack auto-init the rewritten address is whatever happened to be on the stack. Fix this by pre-initializing rtp_addr from the session-level connection address (caddr) when available, and tracking via a have_rtp_addr flag whether any valid address was established. Skip the sdp_session hook entirely when no valid address exists. Fixes: 4ab9e64 ("[NETFILTER]: nf_nat_sip: split up SDP mangling") Reported-by: Xiang Mei <[email protected]> Signed-off-by: Weiming Shi <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent 3db5647 commit 6a2b724

1 file changed

Lines changed: 10 additions & 4 deletions

File tree

net/netfilter/nf_conntrack_sip.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,7 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,
10401040
unsigned int port;
10411041
const struct sdp_media_type *t;
10421042
int ret = NF_ACCEPT;
1043+
bool have_rtp_addr = false;
10431044

10441045
hooks = rcu_dereference(nf_nat_sip_hooks);
10451046

@@ -1056,8 +1057,11 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,
10561057
caddr_len = 0;
10571058
if (ct_sip_parse_sdp_addr(ct, *dptr, sdpoff, *datalen,
10581059
SDP_HDR_CONNECTION, SDP_HDR_MEDIA,
1059-
&matchoff, &matchlen, &caddr) > 0)
1060+
&matchoff, &matchlen, &caddr) > 0) {
10601061
caddr_len = matchlen;
1062+
memcpy(&rtp_addr, &caddr, sizeof(rtp_addr));
1063+
have_rtp_addr = true;
1064+
}
10611065

10621066
mediaoff = sdpoff;
10631067
for (i = 0; i < ARRAY_SIZE(sdp_media_types); ) {
@@ -1091,9 +1095,11 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,
10911095
&matchoff, &matchlen, &maddr) > 0) {
10921096
maddr_len = matchlen;
10931097
memcpy(&rtp_addr, &maddr, sizeof(rtp_addr));
1094-
} else if (caddr_len)
1098+
have_rtp_addr = true;
1099+
} else if (caddr_len) {
10951100
memcpy(&rtp_addr, &caddr, sizeof(rtp_addr));
1096-
else {
1101+
have_rtp_addr = true;
1102+
} else {
10971103
nf_ct_helper_log(skb, ct, "cannot parse SDP message");
10981104
return NF_DROP;
10991105
}
@@ -1125,7 +1131,7 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,
11251131

11261132
/* Update session connection and owner addresses */
11271133
hooks = rcu_dereference(nf_nat_sip_hooks);
1128-
if (hooks && ct->status & IPS_NAT_MASK)
1134+
if (hooks && ct->status & IPS_NAT_MASK && have_rtp_addr)
11291135
ret = hooks->sdp_session(skb, protoff, dataoff,
11301136
dptr, datalen, sdpoff,
11311137
&rtp_addr);

0 commit comments

Comments
 (0)