From: Victor Julien <vjulien@oisf.net>
Date: Thu, 25 Apr 2024 15:07:52 +0000 (+0200)
Subject: detect/iprep: allow 0 as a reputation value
X-Git-Tag: suricata-8.0.0-beta1~1353
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=64dc217f9f64b2e731757ce7bac40cf616e5166c;p=thirdparty%2Fsuricata.git

detect/iprep: allow 0 as a reputation value

Rules would allow checking against value 0, but internally the value
was used to indicate "no value". To address this, the internals now
return negative values for not found. This way value 0 can be fully
supported.

Bug: #6834.
---

diff --git a/src/detect-iprep.c b/src/detect-iprep.c
index b995a238ab..e5069b7072 100644
--- a/src/detect-iprep.c
+++ b/src/detect-iprep.c
@@ -75,65 +75,66 @@ void DetectIPRepRegister (void)
     sigmatch_table[DETECT_IPREP].flags |= SIGMATCH_IPONLY_COMPAT;
 }
 
-static inline uint8_t GetRep(const SReputation *r, const uint8_t cat, const uint32_t version)
+static inline int8_t GetRep(const SReputation *r, const uint8_t cat, const uint32_t version)
 {
     /* allow higher versions as this happens during
      * rule reload */
     if (r != NULL && r->version >= version) {
         return r->rep[cat];
     }
-    return 0;
+    return -1;
 }
 
-static uint8_t GetHostRepSrc(Packet *p, uint8_t cat, uint32_t version)
+/** \returns: -2 no host, -1 no rep entry, 0-127 rep values */
+static int8_t GetHostRepSrc(Packet *p, uint8_t cat, uint32_t version)
 {
     if (p->flags & PKT_HOST_SRC_LOOKED_UP && p->host_src == NULL) {
-        return 0;
+        return -2;
     } else if (p->host_src != NULL) {
         Host *h = (Host *)p->host_src;
         HostLock(h);
         /* use_cnt: 1 for having iprep, 1 for packet ref */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 2);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostUnlock(h);
         return val;
     } else {
         Host *h = HostLookupHostFromHash(&(p->src));
         p->flags |= PKT_HOST_SRC_LOOKED_UP;
         if (h == NULL)
-            return 0;
+            return -2;
         HostReference(&p->host_src, h);
         /* use_cnt: 1 for having iprep, 1 for HostLookupHostFromHash,
          * 1 for HostReference to packet */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 3);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostRelease(h); /* use_cnt >= 2: 1 for iprep, 1 for packet ref */
         return val;
     }
 }
 
-static uint8_t GetHostRepDst(Packet *p, uint8_t cat, uint32_t version)
+static int8_t GetHostRepDst(Packet *p, uint8_t cat, uint32_t version)
 {
     if (p->flags & PKT_HOST_DST_LOOKED_UP && p->host_dst == NULL) {
-        return 0;
+        return -2;
     } else if (p->host_dst != NULL) {
         Host *h = (Host *)p->host_dst;
         HostLock(h);
         /* use_cnt: 1 for having iprep, 1 for packet ref */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 2);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostUnlock(h);
         return val;
     } else {
         Host *h = HostLookupHostFromHash(&(p->dst));
         p->flags |= PKT_HOST_DST_LOOKED_UP;
         if (h == NULL)
-            return 0;
+            return -2;
         HostReference(&p->host_dst, h);
         /* use_cnt: 1 for having iprep, 1 for HostLookupHostFromHash,
          * 1 for HostReference to packet */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 3);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostRelease(h); /* use_cnt >= 2: 1 for iprep, 1 for packet ref */
         return val;
     }
@@ -152,58 +153,58 @@ static int DetectIPRepMatch (DetectEngineThreadCtx *det_ctx, Packet *p,
         return 0;
 
     uint32_t version = det_ctx->de_ctx->srep_version;
-    uint8_t val = 0;
+    int8_t val = 0;
 
     SCLogDebug("rd->cmd %u", rd->cmd);
     switch (rd->cmd) {
         case IPRepCmdAny:
             val = GetHostRepSrc(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepSrc(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                if (DetectU8Match(val, &rd->du8))
+            if (val >= 0) {
+                if (DetectU8Match((uint8_t)val, &rd->du8))
                     return 1;
             }
             val = GetHostRepDst(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepDst(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
 
         case IPRepCmdSrc:
             val = GetHostRepSrc(p, rd->cat, version);
-            SCLogDebug("checking src -- val %u (looking for cat %u, val %u)", val, rd->cat,
+            SCLogDebug("checking src -- val %d (looking for cat %u, val %u)", val, rd->cat,
                     rd->du8.arg1);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepSrc(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
 
         case IPRepCmdDst:
             SCLogDebug("checking dst");
             val = GetHostRepDst(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepDst(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
 
         case IPRepCmdBoth:
             val = GetHostRepSrc(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepSrc(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val == 0 || DetectU8Match(val, &rd->du8) == 0)
+            if (val < 0 || DetectU8Match((uint8_t)val, &rd->du8) == 0)
                 return 0;
             val = GetHostRepDst(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepDst(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
     }
diff --git a/src/reputation.c b/src/reputation.c
index 67a180a324..8533bb0227 100644
--- a/src/reputation.c
+++ b/src/reputation.c
@@ -123,31 +123,31 @@ static void SRepCIDRAddNetblock(SRepCIDRTree *cidr_ctx, char *ip, int cat, uint8
     }
 }
 
-static uint8_t SRepCIDRGetIPv4IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv4_addr, uint8_t cat)
+static int8_t SRepCIDRGetIPv4IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv4_addr, uint8_t cat)
 {
     void *user_data = NULL;
     (void)SCRadixFindKeyIPV4BestMatch(ipv4_addr, cidr_ctx->srepIPV4_tree[cat], &user_data);
     if (user_data == NULL)
-        return 0;
+        return -1;
 
     SReputation *r = (SReputation *)user_data;
     return r->rep[cat];
 }
 
-static uint8_t SRepCIDRGetIPv6IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv6_addr, uint8_t cat)
+static int8_t SRepCIDRGetIPv6IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv6_addr, uint8_t cat)
 {
     void *user_data = NULL;
     (void)SCRadixFindKeyIPV6BestMatch(ipv6_addr, cidr_ctx->srepIPV6_tree[cat], &user_data);
     if (user_data == NULL)
-        return 0;
+        return -1;
 
     SReputation *r = (SReputation *)user_data;
     return r->rep[cat];
 }
 
-uint8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
+int8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
 {
-    uint8_t rep = 0;
+    int8_t rep = -3;
 
     if (PacketIsIPv4(p))
         rep = SRepCIDRGetIPv4IPRep(cidr_ctx, (uint8_t *)GET_IPV4_SRC_ADDR_PTR(p), cat);
@@ -157,9 +157,9 @@ uint8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint
     return rep;
 }
 
-uint8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
+int8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
 {
-    uint8_t rep = 0;
+    int8_t rep = -3;
 
     if (PacketIsIPv4(p))
         rep = SRepCIDRGetIPv4IPRep(cidr_ctx, (uint8_t *)GET_IPV4_DST_ADDR_PTR(p), cat);
diff --git a/src/reputation.h b/src/reputation.h
index 23bc023338..d7009776ce 100644
--- a/src/reputation.h
+++ b/src/reputation.h
@@ -49,8 +49,8 @@ void SRepDestroy(struct DetectEngineCtx_ *de_ctx);
 void SRepReloadComplete(void);
 int SRepHostTimedOut(Host *);
 
-uint8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
-uint8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
+int8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
+int8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
 void SRepResetVersion(void);
 int SRepLoadCatFileFromFD(FILE *fp);
 int SRepLoadFileFromFD(SRepCIDRTree *cidr_ctx, FILE *fp);