From: Shijin Bose (shibose) <shibose@cisco.com>
Date: Wed, 10 Dec 2025 13:11:44 +0000 (+0000)
Subject: Pull request #4985: sip: fix out of bound reads in sip inspector
X-Git-Tag: 3.10.1.0~23
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2628c74552794b24037812a6252406cfcaa9c852;p=thirdparty%2Fsnort3.git

Pull request #4985: sip: fix out of bound reads in sip inspector

Merge in SNORT/snort3 from ~SHIBOSE/snort3:sip_oob to master

Squashed commit of the following:

commit db754babd2279566a1c11267a8249d4f22311467
Author: shibose <shibose@cisco.com>
Date:   Tue Nov 18 12:35:41 2025 +0530

    sip: avoid out-of-bounds reads in sip_parse_sdp_m

commit 0455ee01dad5f61b1eab726facb894f2334615cc
Author: shibose <shibose@cisco.com>
Date:   Fri Nov 7 00:33:16 2025 +0530

    sip: fix out of bound reads in sip inspector
---

diff --git a/src/service_inspectors/sip/sip_parser.cc b/src/service_inspectors/sip/sip_parser.cc
index 1edba4dfe..b6b4d7239 100644
--- a/src/service_inspectors/sip/sip_parser.cc
+++ b/src/service_inspectors/sip/sip_parser.cc
@@ -354,7 +354,7 @@ static bool sip_startline_parse(SIPMsg* msg, const char* buff, const char* end,
             DetectionEngine::queue_event(GID_SIP, SIP_EVENT_INVALID_VERSION);
         }
 
-        space = strchr(buff, ' ');
+        space = (const char*)memchr(buff, ' ', next - buff);
         if (space == nullptr)
             return false;
         statusCode = SnortStrtoul(space + 1, nullptr, 10);
@@ -679,7 +679,7 @@ static int sip_parse_from(SIPMsg* msg, const char* start, const char* end, SIP_P
             msg->dlgID.fromTagHash = strToHash(msg->from_tag,msg->fromTagLen);
             break;
         }
-        buff = (const char*)memchr(buff + 1, ';', msg->fromLen);
+        buff = (const char*)memchr(buff + 1, ';', end - (buff + 1));
     }
 
     userStart = (const char*)memchr(msg->from, ':', msg->fromLen);
@@ -733,7 +733,7 @@ static int sip_parse_to(SIPMsg* msg, const char* start, const char* end, SIP_PRO
             msg->dlgID.toTagHash = strToHash(msg->to_tag,msg->toTagLen);
             break;
         }
-        buff = (const char*)memchr(buff + 1, ';', msg->toLen);
+        buff = (const char*)memchr(buff + 1, ';', end - (buff + 1));
     }
 
     return SIP_PARSE_SUCCESS;
@@ -1114,7 +1114,6 @@ static int sip_parse_sdp_m(SIPMsg* msg, const char* start, const char* end)
 {
     int length;
     const char* spaceIndex;
-    char* next;
     SIP_MediaData* mdata;
 
     if (nullptr == msg->mediaSession)
@@ -1126,16 +1125,46 @@ static int sip_parse_sdp_m(SIPMsg* msg, const char* start, const char* end)
     if ((nullptr == spaceIndex)||(spaceIndex == end))
         return SIP_PARSE_ERROR;
 
+    // Compute bounded token range for the port field: [port_start, token_end)
+    const char* port_start = spaceIndex + 1;
+    if (port_start >= end)
+        return SIP_PARSE_ERROR;
+
+    const char* token_end = (const char*)memchr(port_start, ' ', end - port_start);
+    if (!token_end)
+        token_end = end;
+
+    if (token_end <= port_start)
+        return SIP_PARSE_ERROR;
+
+    // Copy bounded token into an exact-sized, NUL-terminated heap buffer
+    size_t token_len = (size_t)(token_end - port_start);
+    char* buf = (char*)snort_alloc(token_len + 1);
+    memcpy(buf, port_start, token_len);
+    buf[token_len] = '\0';
+
+    // Allocate media data
     mdata = (SIP_MediaData*)snort_calloc(sizeof(SIP_MediaData));
-    mdata->mport = (uint16_t)SnortStrtoul(spaceIndex + 1, &next, 10);
 
-    if ((nullptr != next)&&('/'==next[0]))
-        mdata->numPort = (uint8_t)SnortStrtoul(spaceIndex + 1, &next, 10);
+    // Parse mport from the local, NUL-terminated copy
+    char* next = nullptr;
+    mdata->mport = (uint16_t)SnortStrtoul(buf, &next, 10);
+
+    // Safely check for optional numPort using the local buffer bounds
+    if ((next != nullptr) && (next >= buf) && (next < buf + token_len))
+    {
+        if (*next == '/' && (next + 1) < (buf + token_len))
+        {
+            char* next2 = nullptr;
+            mdata->numPort = (uint8_t)SnortStrtoul(next + 1, &next2, 10);
+        }
+    }
     // Put
     mdata->nextM = msg->mediaSession->medias;
     mdata->maddress = msg->mediaSession->maddress_default;
     msg->mediaSession->medias = mdata;
 
+    snort_free(buf);
     return SIP_PARSE_SUCCESS;
 }