In epaddr_len() and ct_sip_parse_header_uri(), after sip_parse_addr()
successfully parses an IP address, the code checks whether the next
character is ':' to determine if a port number follows. However,
neither function verifies that the pointer is still within bounds
before dereferencing it.

When a SIP header URI contains an IP address that extends to the last
byte of the packet data, in4_pton() or in6_pton() consumes all
available bytes and returns with the end pointer equal to limit. The
subsequent dereference reads one byte past the valid SIP message data.

ct_sip_parse_request() already handles this correctly:

    if (end < limit && *end == ':') {

Apply the same bounds check to the two functions that are missing it.

Fixes: 9fafcd7b2032 ("[NETFILTER]: nf_conntrack/nf_nat: add SIP helper port")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 net/netfilter/nf_conntrack_sip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 939502ff7c87..83741901c6fb 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -194,7 +194,7 @@ static int epaddr_len(const struct nf_conn *ct, const char *dptr,
 	}
 
 	/* Port number */
-	if (*dptr == ':') {
+	if (dptr < limit && *dptr == ':') {
 		dptr++;
 		dptr += digits_len(ct, dptr, limit, shift);
 	}
@@ -520,7 +520,7 @@ int ct_sip_parse_header_uri(const struct nf_conn *ct, const char *dptr,
 
 	if (!sip_parse_addr(ct, dptr + *matchoff, &c, addr, limit, true))
 		return -1;
-	if (*c == ':') {
+	if (c < limit && *c == ':') {
 		c++;
 		p = simple_strtoul(c, (char **)&c, 10);
 		if (p < 1024 || p > 65535)
-- 
2.43.0

Weiming Shi <bestswngs@gmail.com> wrote:
> In epaddr_len() and ct_sip_parse_header_uri(), after sip_parse_addr()
> successfully parses an IP address, the code checks whether the next
> character is ':' to determine if a port number follows. However,
> neither function verifies that the pointer is still within bounds
> before dereferencing it.

I already queued up:
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260313195256.2783257-1-qguanni@gmail.com/

for nf-next (I already sent the 'last' PR for 7.0).

Could you check if that resolves the problem you're reporting?

>  		p = simple_strtoul(c, (char **)&c, 10);

All of these functions require a c-string, which we usually
don't have with network packet parsing.

IOW, sip helper needs to be audited for these problems
but I don't know when I can get to it.

On 26-04-09 17:22, Florian Westphal wrote:
> Weiming Shi <bestswngs@gmail.com> wrote:
> > In epaddr_len() and ct_sip_parse_header_uri(), after sip_parse_addr()
> > successfully parses an IP address, the code checks whether the next
> > character is ':' to determine if a port number follows. However,
> > neither function verifies that the pointer is still within bounds
> > before dereferencing it.
> 
> I already queued up:
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260313195256.2783257-1-qguanni@gmail.com/
> 
> for nf-next (I already sent the 'last' PR for 7.0).
> 
> Could you check if that resolves the problem you're reporting?
> 
> >  		p = simple_strtoul(c, (char **)&c, 10);
> 
> All of these functions require a c-string, which we usually
> don't have with network packet parsing.
> 
> IOW, sip helper needs to be audited for these problems
> but I don't know when I can get to it.

Tested-by: Weiming Shi <bestswngs@gmail.com>