From: Alex Rousskov Date: Wed, 20 May 2026 23:47:25 +0000 (+0000) Subject: SNMP: Improve parsing of OIDs containing IP addresses (#2419) X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=ec328cf16eff35be1a2d938fedc8a1b764ddff10;p=thirdparty%2Fsquid.git SNMP: Improve parsing of OIDs containing IP addresses (#2419) Broken since 2007 commit cc192b50 made a false "4 or 16" assumption, possibly influenced by bad assumptions in earlier client_Inst() code. Also improved const-correctness to get fixed oid2addr() code to compile. --- diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 8e70676885..4509413cee 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -439,7 +439,6 @@ Thank you! Regents of the University of California (UCSD) Reinhard Posmyk Reinhard Sojka - Renaud Metrich Rene Geile Reuben Farrelly Ricardo Ferreira Ribeiro diff --git a/src/client_db.cc b/src/client_db.cc index 21795607ab..385befc84c 100644 --- a/src/client_db.cc +++ b/src/client_db.cc @@ -417,8 +417,8 @@ clientdbStartGC(void) #if SQUID_SNMP -Ip::Address * -client_entry(Ip::Address *current) +const Ip::Address * +client_entry(const Ip::Address *current) { char key[MAX_IPSTRLEN]; hash_first(client_table); @@ -449,9 +449,9 @@ snmp_meshCtblFn(variable_list * Var, snint * ErrP) MemBuf tmp; debugs(49, 6, "Current : length=" << Var->name_length << ": " << snmpDebugOid(Var->name, Var->name_length, tmp)); if (Var->name_length == 16) { - oid2addr(&(Var->name[12]), keyIp, 4); + keyIp = oid2addr(&(Var->name[12]), 4).value(); } else if (Var->name_length == 28) { - oid2addr(&(Var->name[12]), keyIp, 16); + keyIp = oid2addr(&(Var->name[12]), 16).value(); } else { *ErrP = SNMP_ERR_NOSUCHNAME; return nullptr; diff --git a/src/client_db.h b/src/client_db.h index 24b782b563..85ee6cb446 100644 --- a/src/client_db.h +++ b/src/client_db.h @@ -38,7 +38,7 @@ ClientInfo * clientdbGetInfo(const Ip::Address &addr); #endif #if SQUID_SNMP -Ip::Address *client_entry(Ip::Address *current); +const Ip::Address *client_entry(const Ip::Address *); variable_list *snmp_meshCtblFn(variable_list *, snint *); #endif diff --git a/src/redirect.cc b/src/redirect.cc index 528fed105d..14a1027147 100644 --- a/src/redirect.cc +++ b/src/redirect.cc @@ -110,9 +110,8 @@ redirectHandleReply(void *data, const Helper::Reply &reply) // parse it into status=, url= and rewrite-url= keys if (replySize) { MemBuf replyBuffer; - replyBuffer.init(replySize + 1, replySize + 1); // with space for 0-terminator added by append() - Assure(replySize <= size_t(reply.other().contentSize())); - replyBuffer.append(reply.other().content(), replySize); + replyBuffer.init(replySize, replySize); + replyBuffer.append(reply.other().content(), reply.other().contentSize()); char * result = replyBuffer.content(); Helper::Reply newReply; diff --git a/src/snmp_core.cc b/src/snmp_core.cc index 4e06c250ae..4b91a491ff 100644 --- a/src/snmp_core.cc +++ b/src/snmp_core.cc @@ -781,7 +781,7 @@ client_Inst(oid * name, snint * len, mib_tree_entry * current, oid_ParseFn ** Fn { oid *instance = nullptr; Ip::Address laddr; - Ip::Address *aux; + const Ip::Address *aux; int size = 0; int newshift = 0; @@ -807,9 +807,8 @@ client_Inst(oid * name, snint * len, mib_tree_entry * current, oid_ParseFn ** Fn *len += size ; } } else { - int shift = *len - current->len ; // i.e 4 or 16 - oid2addr(&name[*len - shift], laddr,shift); - aux = client_entry(&laddr); + const auto ip = oid2addr(&name[current->len], *len - current->len); + aux = ip ? client_entry(&ip.value()) : nullptr; if (aux) laddr = *aux; else @@ -1048,7 +1047,7 @@ snmpCreateOid(int length,...) * Debug calls, prints out the OID for debugging purposes. */ const char * -snmpDebugOid(oid * Name, snint Len, MemBuf &outbuf) +snmpDebugOid(const oid * const Name, const snint Len, MemBuf &outbuf) { char mbuf[16]; int x; @@ -1105,26 +1104,30 @@ addr2oid(Ip::Address &addr, oid * Dest) oid == 32.1.50.239.162.33.251.20.50.0.0.0.0.0.0.0.0.0.1 ==> IPv6 address : 20:01:32:ef:a2:21:fb:32:00:00:00:00:00:00:00:00:OO:01 */ -void -oid2addr(oid * id, Ip::Address &addr, u_int size) +std::optional +oid2addr(const oid * const id, const size_t size) { + MemBuf tmp; + debugs(49, 7, "oid=" << snmpDebugOid(id, size, tmp)); + struct in_addr i4addr; struct in6_addr i6addr; u_int i; u_char *cp; if ( size == sizeof(struct in_addr) ) cp = (u_char *) &(i4addr.s_addr); - else + else if ( size == sizeof(struct in6_addr) ) cp = (u_char *) &(i6addr); - MemBuf tmp; - debugs(49, 7, "oid2addr: id : " << snmpDebugOid(id, size, tmp) ); + else + return std::nullopt; + for (i=0 ; i + class MemBuf; #define SNMP_REQUEST_SIZE 4096 @@ -47,9 +49,12 @@ AggrType snmpAggrType(oid* Current, snint CurrentLen); extern Comm::ConnectionPointer snmpOutgoingConn; extern PF snmpHandleUdp; -const char * snmpDebugOid(oid * Name, snint Len, MemBuf &outbuf); +const char * snmpDebugOid(const oid *, snint Len, MemBuf &outbuf); void addr2oid(Ip::Address &addr, oid *Dest); -void oid2addr(oid *Dest, Ip::Address &addr, u_int code); + +/// Parses raw OID suffix of a given size as an IP address. +/// \returns nil if the suffix does not represent an IPv4 or IPv6 address +std::optional oid2addr(const oid *, size_t); namespace Acl { diff --git a/src/tests/stub_client_db.cc b/src/tests/stub_client_db.cc index f49a85c762..df521d7685 100644 --- a/src/tests/stub_client_db.cc +++ b/src/tests/stub_client_db.cc @@ -21,7 +21,7 @@ void clientdbSetWriteLimiter(ClientInfo *, const int,const double,const double) ClientInfo *clientdbGetInfo(const Ip::Address &) STUB_RETVAL(nullptr) #endif #if SQUID_SNMP -Ip::Address *client_entry(Ip::Address *) STUB_RETVAL(nullptr) +const Ip::Address *client_entry(const Ip::Address *) STUB_RETVAL(nullptr) variable_list *snmp_meshCtblFn(variable_list *, snint *) STUB_RETVAL(nullptr) #endif