[PATCH] netfilter: nf_conntrack_sip: fix OOB read in SIP URI port parsing

[PATCH] netfilter: nf_conntrack_sip: fix OOB read in SIP URI port parsing

[Jenny Guanni Qu]

In epaddr_len() and ct_sip_parse_header_uri(), after sip_parse_addr()
parses an IP address, the pointer (dptr or c) may point at or past
limit. The subsequent check for a ':' port separator dereferences the
pointer without a bounds check, causing a 1-byte out-of-bounds read.

Add bounds checks before the dereference in both locations.

Fixes: 05e3ced297fe ("[NETFILTER]: nf_conntrack_sip: introduce SIP-URI parsing helper")
Reported-by: Klaudia Kloc <klaudia@vidocsecurity.com>
Reported-by: Dawid Moczadło <dawid@vidocsecurity.com>
Tested-by: Jenny Guanni Qu <qguanni@gmail.com>
Signed-off-by: Jenny Guanni Qu <qguanni@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 d0eac27f6ba0..a232054d7919 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.34.1

Re: [PATCH] netfilter: nf_conntrack_sip: fix OOB read in SIP URI port parsing

[Florian Westphal]

Jenny Guanni Qu <qguanni@gmail.com> wrote:
> In epaddr_len() and ct_sip_parse_header_uri(), after sip_parse_addr()
> parses an IP address, the pointer (dptr or c) may point at or past
> limit. The subsequent check for a ':' port separator dereferences the
> pointer without a bounds check, causing a 1-byte out-of-bounds read.
> 
> Add bounds checks before the dereference in both locations.

I'm not sure.

This bug is real, but I suspect there are many many more bugs in this
code.

> Fixes: 05e3ced297fe ("[NETFILTER]: nf_conntrack_sip: introduce SIP-URI parsing helper")
> Reported-by: Klaudia Kloc <klaudia@vidocsecurity.com>
> Reported-by: Dawid Moczadło <dawid@vidocsecurity.com>
> Tested-by: Jenny Guanni Qu <qguanni@gmail.com>
> Signed-off-by: Jenny Guanni Qu <qguanni@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 d0eac27f6ba0..a232054d7919 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);

I'm not sure this check is enough.  simple_strtoul() assumes
a c-string.  Are we dealing with a c-string here?

I suspect the anser is: 'no' and that we depend on 0 bytes appearing in
skb_shinfo at end of network buffer for this to 'work'.

I would prefer to add much stricter constraints everywhere.

Untested example:
static bool sip_parse_port(const char *dptr, const char **endp, const char *limit)
{
        static const unsigned int max = strlen("65535");
        int len = 0;

        /* port is optional, but dptr >= limit indicates malformed
         * sip message.
         */
        if (dptr >= limit)
                return false;

        if (*dptr != ':') /* this is fine, no port provided. */
                return true;

        while (dptr < limit && isdigit(*dptr)) {
                dptr++;
                len++;

                if (len > max)
                        return false;
        }

        /* reached limit while parsing port */
        if (dptr >= limit)
                return false;

        if (endp)
                *endp = dptr;

        return true;
}

@@ -193,11 +225,9 @@ static int epaddr_len(const struct nf_conn *ct, const char *dptr,
                return 0;
        }
 
-       /* Port number */
-       if (*dptr == ':') {
-               dptr++;
-               dptr += digits_len(ct, dptr, limit, shift);
-       }
+       if (!sip_parse_port(dptr, &dptr, limit))
+               return 0;
+
        return dptr - aux;
 }

Whats your take?

Re: [PATCH] netfilter: nf_conntrack_sip: fix OOB read in SIP URI port parsing

[Guanni Qu]

Hi Florian,

You're right, the minimal bounds check isn't enough. The simple_strtoul()
call assumes a NUL-terminated string, and the SIP packet data in the skb
is not guaranteed to be NUL-terminated. The current code relies on
incidental zero bytes in skb_shinfo.

I like your sip_parse_port() approach. I'll take it, wire it into
both call sites, and send a v2. While I'm in there I'll audit the
rest of nf_conntrack_sip for similar limit violations and send
anything I find as part of the same series.

Jenny

On Thu, Mar 12, 2026 at 8:37 AM Florian Westphal <fw@strlen.de> wrote:
>
> Jenny Guanni Qu <qguanni@gmail.com> wrote:
> > In epaddr_len() and ct_sip_parse_header_uri(), after sip_parse_addr()
> > parses an IP address, the pointer (dptr or c) may point at or past
> > limit. The subsequent check for a ':' port separator dereferences the
> > pointer without a bounds check, causing a 1-byte out-of-bounds read.
> >
> > Add bounds checks before the dereference in both locations.
>
> I'm not sure.
>
> This bug is real, but I suspect there are many many more bugs in this
> code.
>
> > Fixes: 05e3ced297fe ("[NETFILTER]: nf_conntrack_sip: introduce SIP-URI parsing helper")
> > Reported-by: Klaudia Kloc <klaudia@vidocsecurity.com>
> > Reported-by: Dawid Moczadło <dawid@vidocsecurity.com>
> > Tested-by: Jenny Guanni Qu <qguanni@gmail.com>
> > Signed-off-by: Jenny Guanni Qu <qguanni@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 d0eac27f6ba0..a232054d7919 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);
>
> I'm not sure this check is enough.  simple_strtoul() assumes
> a c-string.  Are we dealing with a c-string here?
>
> I suspect the anser is: 'no' and that we depend on 0 bytes appearing in
> skb_shinfo at end of network buffer for this to 'work'.
>
> I would prefer to add much stricter constraints everywhere.
>
> Untested example:
> static bool sip_parse_port(const char *dptr, const char **endp, const char *limit)
> {
>         static const unsigned int max = strlen("65535");
>         int len = 0;
>
>         /* port is optional, but dptr >= limit indicates malformed
>          * sip message.
>          */
>         if (dptr >= limit)
>                 return false;
>
>         if (*dptr != ':') /* this is fine, no port provided. */
>                 return true;
>
>         while (dptr < limit && isdigit(*dptr)) {
>                 dptr++;
>                 len++;
>
>                 if (len > max)
>                         return false;
>         }
>
>         /* reached limit while parsing port */
>         if (dptr >= limit)
>                 return false;
>
>         if (endp)
>                 *endp = dptr;
>
>         return true;
> }
>
> @@ -193,11 +225,9 @@ static int epaddr_len(const struct nf_conn *ct, const char *dptr,
>                 return 0;
>         }
>
> -       /* Port number */
> -       if (*dptr == ':') {
> -               dptr++;
> -               dptr += digits_len(ct, dptr, limit, shift);
> -       }
> +       if (!sip_parse_port(dptr, &dptr, limit))
> +               return 0;
> +
>         return dptr - aux;
>  }
>
> Whats your take?

Re: [PATCH] netfilter: nf_conntrack_sip: fix OOB read in SIP URI port parsing

[Florian Westphal]

Guanni Qu <qguanni@gmail.com> wrote:
> You're right, the minimal bounds check isn't enough. The simple_strtoul()
> call assumes a NUL-terminated string, and the SIP packet data in the skb
> is not guaranteed to be NUL-terminated. The current code relies on
> incidental zero bytes in skb_shinfo.
> 
> I like your sip_parse_port() approach. I'll take it, wire it into
> both call sites, and send a v2. While I'm in there I'll audit the
> rest of nf_conntrack_sip for similar limit violations and send
> anything I find as part of the same series.

Thats great, thanks a lot!